* [FFmpeg-devel] [PATCH v3] avformat/mov: Don't use entry[-1] in pointer arithmetic
@ 2024-02-26 23:27 Andreas Rheinhardt
2024-02-27 0:06 ` James Almer
0 siblings, 1 reply; 3+ messages in thread
From: Andreas Rheinhardt @ 2024-02-26 23:27 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Andreas Rheinhardt
It is undefined behaviour.
Fixes many failed tests with UBSan and GCC 13 like
"src/libavformat/mov.c:4229:44: runtime error: store to address
0x5572abe20f80 with insufficient space for an object of type 'struct
MOVIndexRange'"
(The line number does not refer to the line where &entry[-1]
is assigned.)
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavformat/mov.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/libavformat/mov.c b/libavformat/mov.c
index 71e8f7ae8f..97caaa7723 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -4022,7 +4022,7 @@ static void mov_fix_index(MOVContext *mov, AVStream *st)
int num_discarded_begin = 0;
int first_non_zero_audio_edit = -1;
int packet_skip_samples = 0;
- MOVIndexRange *current_index_range;
+ MOVIndexRange *current_index_range = NULL;
int found_keyframe_after_edit = 0;
int found_non_empty_edit = 0;
@@ -4038,7 +4038,6 @@ static void mov_fix_index(MOVContext *mov, AVStream *st)
return;
}
msc->current_index_range = msc->index_ranges;
- current_index_range = msc->index_ranges - 1;
// Clean AVStream from traces of old index
sti->index_entries = NULL;
@@ -4225,8 +4224,9 @@ static void mov_fix_index(MOVContext *mov, AVStream *st)
}
// Update the index ranges array
- if (current_index_range < msc->index_ranges || index != current_index_range->end) {
- current_index_range++;
+ if (!current_index_range || index != current_index_range->end) {
+ current_index_range = current_index_range ? current_index_range + 1
+ : msc->index_ranges;
current_index_range->start = index;
}
current_index_range->end = index + 1;
@@ -4289,7 +4289,8 @@ static void mov_fix_index(MOVContext *mov, AVStream *st)
av_freep(&frame_duration_buffer);
// Null terminate the index ranges array
- current_index_range++;
+ current_index_range = current_index_range ? current_index_range + 1
+ : msc->index_ranges;
current_index_range->start = 0;
current_index_range->end = 0;
msc->current_index = msc->index_ranges[0].start;
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [FFmpeg-devel] [PATCH v3] avformat/mov: Don't use entry[-1] in pointer arithmetic
2024-02-26 23:27 [FFmpeg-devel] [PATCH v3] avformat/mov: Don't use entry[-1] in pointer arithmetic Andreas Rheinhardt
@ 2024-02-27 0:06 ` James Almer
2024-02-27 11:16 ` Andreas Rheinhardt
0 siblings, 1 reply; 3+ messages in thread
From: James Almer @ 2024-02-27 0:06 UTC (permalink / raw)
To: ffmpeg-devel
On 2/26/2024 8:27 PM, Andreas Rheinhardt wrote:
> It is undefined behaviour.
> Fixes many failed tests with UBSan and GCC 13 like
> "src/libavformat/mov.c:4229:44: runtime error: store to address
> 0x5572abe20f80 with insufficient space for an object of type 'struct
> MOVIndexRange'"
> (The line number does not refer to the line where &entry[-1]
> is assigned.)
>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
> libavformat/mov.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index 71e8f7ae8f..97caaa7723 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -4022,7 +4022,7 @@ static void mov_fix_index(MOVContext *mov, AVStream *st)
> int num_discarded_begin = 0;
> int first_non_zero_audio_edit = -1;
> int packet_skip_samples = 0;
> - MOVIndexRange *current_index_range;
> + MOVIndexRange *current_index_range = NULL;
> int found_keyframe_after_edit = 0;
> int found_non_empty_edit = 0;
>
> @@ -4038,7 +4038,6 @@ static void mov_fix_index(MOVContext *mov, AVStream *st)
> return;
> }
> msc->current_index_range = msc->index_ranges;
> - current_index_range = msc->index_ranges - 1;
>
> // Clean AVStream from traces of old index
> sti->index_entries = NULL;
> @@ -4225,8 +4224,9 @@ static void mov_fix_index(MOVContext *mov, AVStream *st)
> }
>
> // Update the index ranges array
> - if (current_index_range < msc->index_ranges || index != current_index_range->end) {
> - current_index_range++;
> + if (!current_index_range || index != current_index_range->end) {
> + current_index_range = current_index_range ? current_index_range + 1
> + : msc->index_ranges;
> current_index_range->start = index;
> }
> current_index_range->end = index + 1;
> @@ -4289,7 +4289,8 @@ static void mov_fix_index(MOVContext *mov, AVStream *st)
> av_freep(&frame_duration_buffer);
>
> // Null terminate the index ranges array
> - current_index_range++;
> + current_index_range = current_index_range ? current_index_range + 1
> + : msc->index_ranges;
> current_index_range->start = 0;
> current_index_range->end = 0;
> msc->current_index = msc->index_ranges[0].start;
Can't test if this fixes the UB, but the changes lgtm.
_______________________________________________
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".
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [FFmpeg-devel] [PATCH v3] avformat/mov: Don't use entry[-1] in pointer arithmetic
2024-02-27 0:06 ` James Almer
@ 2024-02-27 11:16 ` Andreas Rheinhardt
0 siblings, 0 replies; 3+ messages in thread
From: Andreas Rheinhardt @ 2024-02-27 11:16 UTC (permalink / raw)
To: ffmpeg-devel
James Almer:
> On 2/26/2024 8:27 PM, Andreas Rheinhardt wrote:
>> It is undefined behaviour.
>> Fixes many failed tests with UBSan and GCC 13 like
>> "src/libavformat/mov.c:4229:44: runtime error: store to address
>> 0x5572abe20f80 with insufficient space for an object of type 'struct
>> MOVIndexRange'"
>> (The line number does not refer to the line where &entry[-1]
>> is assigned.)
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
>> ---
>> libavformat/mov.c | 11 ++++++-----
>> 1 file changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/libavformat/mov.c b/libavformat/mov.c
>> index 71e8f7ae8f..97caaa7723 100644
>> --- a/libavformat/mov.c
>> +++ b/libavformat/mov.c
>> @@ -4022,7 +4022,7 @@ static void mov_fix_index(MOVContext *mov,
>> AVStream *st)
>> int num_discarded_begin = 0;
>> int first_non_zero_audio_edit = -1;
>> int packet_skip_samples = 0;
>> - MOVIndexRange *current_index_range;
>> + MOVIndexRange *current_index_range = NULL;
>> int found_keyframe_after_edit = 0;
>> int found_non_empty_edit = 0;
>> @@ -4038,7 +4038,6 @@ static void mov_fix_index(MOVContext *mov,
>> AVStream *st)
>> return;
>> }
>> msc->current_index_range = msc->index_ranges;
>> - current_index_range = msc->index_ranges - 1;
>> // Clean AVStream from traces of old index
>> sti->index_entries = NULL;
>> @@ -4225,8 +4224,9 @@ static void mov_fix_index(MOVContext *mov,
>> AVStream *st)
>> }
>> // Update the index ranges array
>> - if (current_index_range < msc->index_ranges || index !=
>> current_index_range->end) {
>> - current_index_range++;
>> + if (!current_index_range || index !=
>> current_index_range->end) {
>> + current_index_range = current_index_range ?
>> current_index_range + 1
>> + :
>> msc->index_ranges;
>> current_index_range->start = index;
>> }
>> current_index_range->end = index + 1;
>> @@ -4289,7 +4289,8 @@ static void mov_fix_index(MOVContext *mov,
>> AVStream *st)
>> av_freep(&frame_duration_buffer);
>> // Null terminate the index ranges array
>> - current_index_range++;
>> + current_index_range = current_index_range ? current_index_range + 1
>> + : msc->index_ranges;
>> current_index_range->start = 0;
>> current_index_range->end = 0;
>> msc->current_index = msc->index_ranges[0].start;
>
> Can't test if this fixes the UB, but the changes lgtm.
It does:
https://fate.ffmpeg.org/report.cgi?slot=x86_64-archlinux-gcc-ubsan&time=20240227105551
- Andreas
_______________________________________________
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".
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-02-27 11:15 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-26 23:27 [FFmpeg-devel] [PATCH v3] avformat/mov: Don't use entry[-1] in pointer arithmetic Andreas Rheinhardt
2024-02-27 0:06 ` James Almer
2024-02-27 11:16 ` Andreas Rheinhardt
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