michaelni changed the topic of #ffmpeg-devel to: Welcome to the FFmpeg development channel | Questions about using FFmpeg or developing with libav* libs should be asked in #ffmpeg | This channel is publicly logged | FFmpeg 7.1 has been released! | Please read ffmpeg.org/developer.html#Code-of-conduct
<compnn>
names_are_hard, can you share the sample on trac or is it already there ?
<names_are_hard>
Haven't yet - not sure it's useful. Might be my local changes causing the problem
<compnn>
no worries :)
<compnn>
i wish we could make asserts for every function . run scans on all of our videos to get the min/max values then lock em in :D
<names_are_hard>
It would be plausible to automate adding asserts for null pointer params, at least
<names_are_hard>
Harder with local vars I guess. Maybe some clang AST magic could do it
<compnn>
i dont think anyone would much agree with asserting every piece of every codec :D
<names_are_hard>
I am well aware that time is limited and there are always priorities, so, eh
<names_are_hard>
Yeah but it would only assert on debug builds. And you have a pretty good test set
<BtbN>
The ideal solution would be some system for a function to declare that some parameter must not be null and/or within a certain range
<BtbN>
and then the compiler checks for those conditions being pre-checked or forwarded
<BtbN>
but I don't think anything like that exists in C, even as extension
<names_are_hard>
That post seems to say asserts are free if you only enable them in debug builds
<names_are_hard>
Would you get value with your code by enabling asserts in a build you never ship? Up to you
_whitelogger_ has quit [Remote host closed the connection]
_whitelogger_ has joined #ffmpeg-devel
wyatt8740 has quit [Ping timeout: 252 seconds]
iive has quit [Quit: They came for me...]
thilo has quit [Ping timeout: 252 seconds]
thilo has joined #ffmpeg-devel
arch1t3cht8 has joined #ffmpeg-devel
arch1t3cht has quit [Ping timeout: 252 seconds]
arch1t3cht8 is now known as arch1t3cht
Mirarora has quit [Quit: Mirarora encountered a fatal error and needs to close]
Mirarora has joined #ffmpeg-devel
_whitelogger_ has quit [Remote host closed the connection]
_whitelogger_ has joined #ffmpeg-devel
_whitelogger_ has quit [Remote host closed the connection]
_whitelogger_ has joined #ffmpeg-devel
jamrial has quit []
_whitelogger_ has quit [Remote host closed the connection]
_whitelogger_ has joined #ffmpeg-devel
System_Error has quit [Ping timeout: 260 seconds]
Martchus_ has joined #ffmpeg-devel
^Neo has quit [Ping timeout: 252 seconds]
Martchus has quit [Ping timeout: 252 seconds]
System_Error has joined #ffmpeg-devel
<names_are_hard>
Confirmed the crash is not related to my changes, but a problem in jpeg parsing. I've emailed ffmpeg-security@ffmpeg.org, this includes a sample
<names_are_hard>
Reliably crashes Debian ffplay and ffmpeg. They're 7.1 something. If you support platforms where the 0 page is mapped and writable, there's some possibility this could be used for RCE
<names_are_hard>
But I haven't investigated what control you can have over the data that gets written
_whitelogger_ has quit [Remote host closed the connection]
<names_are_hard>
Do you care about malformed samples that trigger asserts? E.g. "Assertion frame->format != AV_SAMPLE_FMT_NONE && frame->sample_rate > 0 && frame->ch_layout.nb_channels > 0 failed at src/fftools/ffmpeg_enc.c:219"
<names_are_hard>
Some of them trigger "[adpcm_ima_apc @ 0x56197ede95c0] An invalid frame was output by a decoder. This is a bug, please report it.", some of them don't. Same assert
<JEEB>
it depends; some such things show issues with implementation like the decoder, others would not lead to changes
<names_are_hard>
Thanks. Some projects only care about normal files causing asserts, which is fair enough. Is there an easy way for me to tell which you want?
<names_are_hard>
Related question, different files - do you care about small files that take a disproportionate time to process? E.g. I have a 19 byte file that takes 8s to decode to null with ffmpeg
<JEEB>
if it's valid there's more chances of care, if not then less
<elenril>
names_are_hard: any assert that can be triggerd by valid api use is a bug
<names_are_hard>
Cool, will send them all to Trac
kasper93 has joined #ffmpeg-devel
cone-379 has quit [Quit: transmission timeout]
jamrial has joined #ffmpeg-devel
_whitelogger_ has quit [Remote host closed the connection]
^Neo has joined #ffmpeg-devel
^Neo has quit [Changing host]
^Neo has joined #ffmpeg-devel
<j-b>
michaelni: when do you tag 7.1.1 ?
_whitelogger_ has joined #ffmpeg-devel
cone-029 has joined #ffmpeg-devel
<cone-029>
ffmpeg tangsha master:8a5b74f98b33: delete unused variable ret
Mirarora has quit [Quit: Mirarora encountered a fatal error and needs to close]
MisterMinister has quit [Ping timeout: 252 seconds]
<cone-029>
ffmpeg Rémi Denis-Courmont master:bbb0fdedb78c: lavc/h264idct: fix RISC-V group multiplier
<cone-029>
ffmpeg Rémi Denis-Courmont release/7.1:f686cf77db3d: lavc/h264idct: fix RISC-V group multiplier
<michaelni>
j-b, last days have been quite alot of work, i can tag it anytime people want of course but idealy i should backport all fixes from master and maybe check if there are important sec issues that need fixing and fix them first
mkver has quit [Ping timeout: 248 seconds]
_whitelogger_ has quit [Remote host closed the connection]
_whitelogger_ has joined #ffmpeg-devel
<kurosu>
So maybe the topic should be put to rest, but people here probably meant they wanted to catch that NULL pointer that doesn't belong, as early as possible, and not when it would cause a crash. eg, if some other code path is added, that wouldn't help. And I guess the earlier you catch that, the more likely the library can return an error due to (internal/API) user input
<BBB>
I would put it to rest :)
<courmisch>
pheww, finally a board can run FATE with proper runtime CPU detection
<compnn>
names_are_hard, to specify further, we'll take any and all fuzz samples that show bugs. for sure.
<cone-029>
ffmpeg James Almer release/7.1:779b0fe015b1: avformat/mov: factorize getting the current item
<cone-029>
ffmpeg James Almer release/7.1:23697c3f02c4: avformat/mov: split off heif item initialization to its own function
<cone-029>
ffmpeg James Almer release/7.1:f8fcebae95f7: avformat/mov: use an array of pointers for heif_item
<cone-029>
ffmpeg James Almer release/7.1:5f8b02a9ffc2: avcodec/aom_film_grain: allocate film grain metadata dynamically
<cone-029>
ffmpeg James Almer release/7.1:b9abdd9eaa4a: avcodec/h2645_sei: use the RefStruct API for film_grain_characteristics
<cone-029>
ffmpeg James Almer release/7.1:fa15e3839dae: avfilter/framepool: align the frame buffers
<cone-029>
ffmpeg Pavel Koshevoy release/7.1:85f389520dbd: lavfi/vf_zscale: fix tmp buffer ptr alignment for zimg_filter_graph_process
<cone-029>
ffmpeg Pavel Koshevoy release/7.1:5b461ffb041e: lavfi/vf_zscale: fix call to av_pix_fmt_count_planes
<cone-029>
ffmpeg James Almer release/7.1:99f6adce6072: avfilter/vf_zscale: align the frame buffers
<j-b>
jamrial: NICE, thx for the backport
<j-b>
I need to check the VVC ones
<j-b>
Nice, it's in too.
<names_are_hard>
compnn: yup, good to know - my questions are really "what counts as a bug?". Slow malformed samples are really borderline. Maybe it indicates excessive looping or memory usage, maybe it's just "file said it's 2000 minutes long"
<compnn>
well if file reports 2000 minutes long, then file does it. if ffplay/ffmpeg try to process it for 2000 minutes, then thats the bug :D
<names_are_hard>
Then I guess I'll Trac all the slow crap and you can determine if you care :) I will not be a pushy noob who claims everything is Critical ;)
<compnn>
thanks
Marth64 has joined #ffmpeg-devel
<compnn>
ffmpeg has been extensively fuzz tested. apparently we are important project to fuzz
<names_are_hard>
Fuzz testing is tricky. You're in Googles server, right?
<names_are_hard>
Yeah
<names_are_hard>
Makes sense, handling this many file formats is a bitch
<compnn>
not me in particular but ive seen it / coverity
<names_are_hard>
But, I found a crash bug in less than one hour, on a single core on a laptop
<names_are_hard>
So I feel the fuzz strategy is not optimal
<BtbN>
Cause you exposed it with a patch first, no?
<names_are_hard>
Yes and no - it's in vanilla
mkver has joined #ffmpeg-devel
<names_are_hard>
Fuzzing will tend to search width first (not *exclusively*, but it's kind of the default). I forced depth in LJ92 area, with a custom mutator for AFL. I guess I'd recommend if you add support for new things, force some deep fuzzing in that area
<names_are_hard>
I see you have some libfuzzer integration, and I think you get free server time from Google to fuzz? But, I didn't see anything in the code that suggests you have builds designed for fuzzing
<names_are_hard>
E.g. see __AFL_LOOP, which can make fuzzing 10x or even 100x more time efficient, but needs someone with knowledge of the internals to put the macro in the right place
<names_are_hard>
I gave it a try, but it didn't improve speed. I'm stuck at about 140 iterations per sec per core, which is okay but not great
<fflogger>
[editedticket] courmisch: Ticket #11302 ([avcodec] RISCV: Properly set relocate parameter of const macro in libavcodec/riscv/h264dsp_rvv.S) updated https://trac.ffmpeg.org/ticket/11302#comment:4
<names_are_hard>
I don't have the spare time to learn ffmpeg code, it's complex! I'm doing this because I want to improve MLV support (I am a dev for that project). I may as well run some fuzzing for a few weeks, maybe it'll demonstrate there's value there for someone to pick up
<compnn>
no worries. i'm just trying to figure out if you need anything :D
<names_are_hard>
Double whisky and some nachos, thanks
<compnn>
dont even think i've ran across MLV files
<compnn>
which is weird for me
<names_are_hard>
But I think I am fine for now - ffmpeg security email hasn't replied yet but it's only been 12 hours. Trac stuff will happen when I find time to bundle up samples by assert message and submit into tickets
<names_are_hard>
Yeah, MLV is suuuuuper niche
<names_are_hard>
Homebrew format for dumping raw sensor data from Canon cams into video, via 3rd party open source software that's been hacked onto the cams
<names_are_hard>
Avoids all mov / mp4 restrictions to bitrate, file length etc, at the expense of enormous file size
<names_are_hard>
New seg fault. Looks like an OOB heap write in vc1 decoding. Valgrind says "Invalid write of size 2", "Address 0x1415fe30 is 16 bytes before a block of size 6 alloc'd". Writes before blocks are very often exploitable
<names_are_hard>
Will email security address
Sean_McG has joined #ffmpeg-devel
<Sean_McG>
hi
<Sean_McG>
I don't know what to do about my swscale vs. AltiVec patch -- I don't really understand what is going on (swscale is out-of-my-depth) but my end goal is that I need Debian-MM to re-enable AltiVec & VSX in their builds. It's not all broken but it was recently disabled wholesale on the assertion that we have been unable to fix the broken accelerations
<courmisch>
can't you just turn off whatever is broken
DauntlessOne has quit [Read error: Connection reset by peer]
<Sean_McG>
not everything is broken, just a few things here-and-there that FATE points out regularly.
<courmisch>
yes, so?
<Sean_McG>
courmisch: I could, but I would be effective #if 0-ing them
<Sean_McG>
*effectively
<courmisch>
and?
<Sean_McG>
I got the impression it would get shot down in review
DauntlessOne has joined #ffmpeg-devel
Everything has joined #ffmpeg-devel
<Sean_McG>
OK, I'll submit them later and see where they go. Heading out to watch a football game soon.
___nick___ has joined #ffmpeg-devel
<Sean_McG>
I have a few minutes now -- is there a way for me to mark the accelerations as "experimental" in the off chance someone wants to enable the forcibly but they remain off by default?
<BBB>
I don't believe so
<Sean_McG>
OK.
<BBB>
we just have bitexact, which is more the opposite
<BBB>
on-by-default but possible to disable forcibly
<BBB>
off-by-default but possible to enable forcibly isn't a concept we have right now, I'm afraid
<Sean_McG>
fair enough
<wbs>
BBB: in swscale, bitexact isn't enabled by default
<BBB>
o_O oh really I didn't know that
<BBB>
wait, you mean, the bitexact flag is not enabled by default
<wbs>
yes
<BBB>
I knew that :)
<BBB>
I know bitexact is that, but it allows function assignment to be the opposite
<BBB>
he's looking for function assignment like bitexact-flag
<BBB>
which requires a bitexact-like-flag that does the opposite
<wbs>
yeah
<Sean_McG>
I think it would semantically incorrect for me to use the bit-exact flag for this
<BBB>
agreed
<Sean_McG>
OK, #if 0 with a comment it is then
q66 has quit [Ping timeout: 252 seconds]
<wbs>
wrt ppc assembly, I would expect that much of it may have been written for big-endian ppc many years ago, and/or might have assumptions that don't hold up on modern power
<Sean_McG>
many of them functioned correctly as written on little-endian as well, and some of them were even adapted and/or rewritten for VSX
Everything has quit [Quit: leaving]
<Sean_McG>
there's just a few that have been regularly or sporadically failing on FATE
<Sean_McG>
which remind me -- the lossless audio checkasm only seems to fail for large random seeds. We've fixed similar issues before but is there somewhere to look first when that happens?
<wbs>
it's probably not specifically for large vs small seeds, it's just that different seeds generate different input data
<Sean_McG>
but the checkasm test is only comparing the result between the C version and the accelerated one
<wbs>
if the assembly works fine for actual decoding, it can be that the checkasm test generates input data which is out of the range for real world cases
<Sean_McG>
which I assume they share a seed
<wbs>
of course they share a seed
<wbs>
or they are run with various seeds, but when comparing, it generates 1 set of input data with some seed, then pass that same random input data to both C and simd version, and compare the output
<wbs>
but say that you take int16_t inputs, but the actual valid range of values only is signed 12 bit, then the simd implementation may be totally ok, but if you pass it values outside of the range, it can very well produce a different output than the C code
<wbs>
it can either be a checkasm bug, or an asm bug - which of them it is needs to be debugged and figured out
<frankplow>
names_are_hard: There is a "custom build for fuzzing" as you put it under tools/target_*_fuzzer.c. I think this is already set up to be what AFL calls "persistent mode" that you get with __AFL_LOOP, just the syntax is a bit different for libfuzzer.
<frankplow>
But yes I agree re. the depth vs breadth point. If you pick a particular component and only instrument that, then I think you will probably end up catching bugs that aren't caught by OSSFuzz, just because the resources there are spread over so many components. Providing a large corpus of samples for a particular component helps as well, I think OSSFuzz uses the samples from FATE where the size is limited due to runtime requirements.
johnny__ has quit [Remote host closed the connection]
<fflogger>
[editedticket] MasterQuestionable: Ticket #8385 ([avformat] libavformat/aviobuf: A part of conditional expression is always true: whence != 2) updated https://trac.ffmpeg.org/ticket/8385#comment:4
<fflogger>
[editedticket] Balling: Ticket #8385 ([avformat] libavformat/aviobuf: A part of conditional expression is always true: whence != 2) updated https://trac.ffmpeg.org/ticket/8385#comment:5