Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
From: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
To: ffmpeg-devel@ffmpeg.org
Subject: Re: [FFmpeg-devel] [PATCH] avformat/vpk: fix divide by zero
Date: Sat, 10 Aug 2024 11:25:38 +0200
Message-ID: <DU0P250MB0747EC57F8AECD18700FBE008FBB2@DU0P250MB0747.EURP250.PROD.OUTLOOK.COM> (raw)
In-Reply-To: <CABPLASQdGJYmxRNx6imnadzcr1kqkN_9Bu+gF_SxTzdMCiOmFw@mail.gmail.com>

Kacper Michajlow:
> On Fri, 9 Aug 2024 at 22:51, Michael Niedermayer <michael@niedermayer.cc> wrote:
>>
>> On Wed, Aug 07, 2024 at 03:42:46PM +0200, Kacper Michajłow wrote:
>>> Can happen after calling avformat_find_stream_info() when the codec
>>> fails to open, but return value is 0 and subsequent uses of this context
>>> have zero value in channel number.
>>>
>>> Found by OSS-Fuzz.
>>>
>>> Signed-off-by: Kacper Michajłow <kasper93@gmail.com>
>>> ---
>>>  libavformat/vpk.c | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/libavformat/vpk.c b/libavformat/vpk.c
>>> index 001ad33555..aa98ef2dd4 100644
>>> --- a/libavformat/vpk.c
>>> +++ b/libavformat/vpk.c
>>> @@ -86,6 +86,8 @@ static int vpk_read_packet(AVFormatContext *s, AVPacket *pkt)
>>>
>>>      vpk->current_block++;
>>>      if (vpk->current_block == vpk->block_count) {
>>> +        if (par->ch_layout.nb_channels <= 0)
>>> +            return AVERROR_INVALIDDATA;
>>>          unsigned size = vpk->last_block_size / par->ch_layout.nb_channels;
>>>          unsigned skip = (par->block_align - vpk->last_block_size) / par->ch_layout.nb_channels;
>>>          uint64_t pos = avio_tell(s->pb);
>>
>> iam not sure if a parser or other should replace a valid set of
>> parameters by an invalid
>> (this patch implies that such a action occured)
>>
>> can you explain more detailedly by what and why channels is set to 0 ?
>>
> 
> You are right, it might be better to improve this to not override the
> params. Let me explain what happens, I didn't read through the whole
> avformat_find_stream_info() to know what would be the best approach
> yet. I will try to look at it, but if you have immediate ideas, that
> would be nice.
> 
> 1. avformat_open_input() sets nb_channels to 108
> 
> 2. Just after that we call avformat_find_stream_info(avfc, NULL); this
> returns 0 (success), but as a result it overrides params already
> present in the context.
> log for reference, during the find stream info call
> [ffmpeg/demuxer] vpk: Before avformat_find_stream_info() pos:
> 538976288 bytes read:21 seeks:1 nb_streams:1
> [ffmpeg/demuxer] vpk: Failed to open codec in avformat_find_stream_info
> [lavf] mp_seek(0x512000018090, 0, size)
> [lavf] 0=mp_read(0x512000018090, 0x7fe4c7ce8800, 50000000), pos:
> 538976288, eof:1
> [lavf] 0=mp_read(0x512000018090, 0x52d00000a400, 32768), pos: 538976288, eof:1
> [ffmpeg/audio] adpcm_psx: Decoder requires channel layout to be set
> [ffmpeg/demuxer] vpk: Failed to open codec in avformat_find_stream_info
> [lavf] mp_seek(0x512000018090, 0, size)
> [lavf] mp_seek(0x512000018090, 0, size)
> [lavf] mp_seek(0x512000018090, 0, size)
> [ffmpeg/demuxer] vpk: stream 0: start_time: NOPTS duration: 0.069852
> [ffmpeg/demuxer] vpk: format: start_time: NOPTS duration: 0.069852
> (estimate from stream) bitrate=2 kb/s
> [ffmpeg/demuxer] vpk: Could not find codec parameters for stream 0
> (Audio: adpcm_psx, 538976288 Hz, 0 channels): unspecified sample
> format
> [ffmpeg/demuxer] Consider increasing the value for the
> 'analyzeduration' (0) and 'probesize' (5000000) options
> [ffmpeg/demuxer] vpk: After avformat_find_stream_info() pos: 538976288
> bytes read:21 seeks:1 frames:0
> 
> 3. the nb_channels value is cleared in avformat_find_stream_info() ->
> avcodec_parameters_from_context() -> codec_parameters_reset() and
> remains 0.

This seems like the error: Why is AVCodecParameters being set from an
AVCodecContext if the codec could not be successfully opened?

> 
> 4. as we can see there were errors, but it still returns success, so we proceed.
> 
> 5. on the next av_seek_frame() which goes to vpk_read_packet() it will
> fail because nb_channels is 0 at this point.
> 
> Sorry for only a high level overview, but at this moment, I'm not sure
> how exactly it is supposed to work. I thought it might be intended to
> override headers parameters later if we know better later on, that's
> why my initial patch only tackled this case.
> 

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

  reply	other threads:[~2024-08-10  9:25 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-07 13:42 Kacper Michajłow
2024-08-09 20:42 ` Michael Niedermayer
2024-08-09 23:51   ` Kacper Michajlow
2024-08-10  9:25     ` Andreas Rheinhardt [this message]
2024-08-10 16:49       ` Kacper Michajlow

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DU0P250MB0747EC57F8AECD18700FBE008FBB2@DU0P250MB0747.EURP250.PROD.OUTLOOK.COM \
    --to=andreas.rheinhardt@outlook.com \
    --cc=ffmpeg-devel@ffmpeg.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git