| Message ID | 20200303042006.6370-2-andreas.rheinhardt@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [FFmpeg-devel,1/2] avcodec/v4l2_m2m: Avoid using intermediate buffer | expand |
| Context | Check | Description |
|---|---|---|
| andriy/ffmpeg-patchwork | success | Make fate finished |
Andreas Rheinhardt: > If an URI indicated that the data protocol was in use, it would be > copied into a temporary buffer via strncpy(dst, src, strlen(src)), > thereby ensuring that the trailing \0 would not be copied, despite dst > being uninitialized. dst would then be av_strdup'ed, leading to > potential segfaults. > > The solution to this is simple: Don't copy the URI in the temporary > buffer at all, instead av_strdup it directly. > > This fixes a -Wstringop-truncation warning emitted by GCC 9.2. > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> > --- > This is honestly untested, as this is not covered by FATE. > > libavformat/hls.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/libavformat/hls.c b/libavformat/hls.c > index 1f58e745a7..fc45719d1c 100644 > --- a/libavformat/hls.c > +++ b/libavformat/hls.c > @@ -403,8 +403,7 @@ static struct segment *new_init_section(struct playlist *pls, > const char *url_base) > { > struct segment *sec; > - char *ptr; > - char tmp_str[MAX_URL_SIZE]; > + char tmp_str[MAX_URL_SIZE], *ptr = tmp_str; > > if (!info->uri[0]) > return NULL; > @@ -414,11 +413,11 @@ static struct segment *new_init_section(struct playlist *pls, > return NULL; > > if (!av_strncasecmp(info->uri, "data:", 5)) { > - strncpy(tmp_str, info->uri, strlen(info->uri)); > + ptr = info->uri; > } else { > ff_make_absolute_url(tmp_str, sizeof(tmp_str), url_base, info->uri); > } > - sec->url = av_strdup(tmp_str); > + sec->url = av_strdup(ptr); > if (!sec->url) { > av_free(sec); > return NULL; > Ping. - Andreas
> 2020年3月22日 上午7:28，Andreas Rheinhardt <andreas.rheinhardt@gmail.com> 写道： > > Andreas Rheinhardt: >> If an URI indicated that the data protocol was in use, it would be >> copied into a temporary buffer via strncpy(dst, src, strlen(src)), >> thereby ensuring that the trailing \0 would not be copied, despite dst >> being uninitialized. dst would then be av_strdup'ed, leading to >> potential segfaults. >> >> The solution to this is simple: Don't copy the URI in the temporary >> buffer at all, instead av_strdup it directly. >> >> This fixes a -Wstringop-truncation warning emitted by GCC 9.2. >> >> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> >> --- >> This is honestly untested, as this is not covered by FATE. >> >> libavformat/hls.c | 7 +++---- >> 1 file changed, 3 insertions(+), 4 deletions(-) >> >> diff --git a/libavformat/hls.c b/libavformat/hls.c >> index 1f58e745a7..fc45719d1c 100644 >> --- a/libavformat/hls.c >> +++ b/libavformat/hls.c >> @@ -403,8 +403,7 @@ static struct segment *new_init_section(struct playlist *pls, >> const char *url_base) >> { >> struct segment *sec; >> - char *ptr; >> - char tmp_str[MAX_URL_SIZE]; >> + char tmp_str[MAX_URL_SIZE], *ptr = tmp_str; >> >> if (!info->uri[0]) >> return NULL; >> @@ -414,11 +413,11 @@ static struct segment *new_init_section(struct playlist *pls, >> return NULL; >> >> if (!av_strncasecmp(info->uri, "data:", 5)) { >> - strncpy(tmp_str, info->uri, strlen(info->uri)); >> + ptr = info->uri; >> } else { >> ff_make_absolute_url(tmp_str, sizeof(tmp_str), url_base, info->uri); >> } >> - sec->url = av_strdup(tmp_str); >> + sec->url = av_strdup(ptr); >> if (!sec->url) { >> av_free(sec); >> return NULL; >> > Ping. Looks ok. > > - 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". Thanks Steven Liu
Steven Liu: > > >> 2020年3月22日 上午7:28，Andreas Rheinhardt <andreas.rheinhardt@gmail.com> 写道： >> >> Andreas Rheinhardt: >>> If an URI indicated that the data protocol was in use, it would be >>> copied into a temporary buffer via strncpy(dst, src, strlen(src)), >>> thereby ensuring that the trailing \0 would not be copied, despite dst >>> being uninitialized. dst would then be av_strdup'ed, leading to >>> potential segfaults. >>> >>> The solution to this is simple: Don't copy the URI in the temporary >>> buffer at all, instead av_strdup it directly. >>> >>> This fixes a -Wstringop-truncation warning emitted by GCC 9.2. >>> >>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> >>> --- >>> This is honestly untested, as this is not covered by FATE. >>> >>> libavformat/hls.c | 7 +++---- >>> 1 file changed, 3 insertions(+), 4 deletions(-) >>> >>> diff --git a/libavformat/hls.c b/libavformat/hls.c >>> index 1f58e745a7..fc45719d1c 100644 >>> --- a/libavformat/hls.c >>> +++ b/libavformat/hls.c >>> @@ -403,8 +403,7 @@ static struct segment *new_init_section(struct playlist *pls, >>> const char *url_base) >>> { >>> struct segment *sec; >>> - char *ptr; >>> - char tmp_str[MAX_URL_SIZE]; >>> + char tmp_str[MAX_URL_SIZE], *ptr = tmp_str; >>> >>> if (!info->uri[0]) >>> return NULL; >>> @@ -414,11 +413,11 @@ static struct segment *new_init_section(struct playlist *pls, >>> return NULL; >>> >>> if (!av_strncasecmp(info->uri, "data:", 5)) { >>> - strncpy(tmp_str, info->uri, strlen(info->uri)); >>> + ptr = info->uri; >>> } else { >>> ff_make_absolute_url(tmp_str, sizeof(tmp_str), url_base, info->uri); >>> } >>> - sec->url = av_strdup(tmp_str); >>> + sec->url = av_strdup(ptr); >>> if (!sec->url) { >>> av_free(sec); >>> return NULL; >>> >> Ping. > Looks ok. >> >> - 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". > > Thanks > > Steven Liu > Applied, thanks. - Andreas
diff --git a/libavformat/hls.c b/libavformat/hls.c index 1f58e745a7..fc45719d1c 100644 --- a/libavformat/hls.c +++ b/libavformat/hls.c @@ -403,8 +403,7 @@ static struct segment *new_init_section(struct playlist *pls, const char *url_base) { struct segment *sec; - char *ptr; - char tmp_str[MAX_URL_SIZE]; + char tmp_str[MAX_URL_SIZE], *ptr = tmp_str; if (!info->uri[0]) return NULL; @@ -414,11 +413,11 @@ static struct segment *new_init_section(struct playlist *pls, return NULL; if (!av_strncasecmp(info->uri, "data:", 5)) { - strncpy(tmp_str, info->uri, strlen(info->uri)); + ptr = info->uri; } else { ff_make_absolute_url(tmp_str, sizeof(tmp_str), url_base, info->uri); } - sec->url = av_strdup(tmp_str); + sec->url = av_strdup(ptr); if (!sec->url) { av_free(sec); return NULL;
If an URI indicated that the data protocol was in use, it would be copied into a temporary buffer via strncpy(dst, src, strlen(src)), thereby ensuring that the trailing \0 would not be copied, despite dst being uninitialized. dst would then be av_strdup'ed, leading to potential segfaults. The solution to this is simple: Don't copy the URI in the temporary buffer at all, instead av_strdup it directly. This fixes a -Wstringop-truncation warning emitted by GCC 9.2. Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> --- This is honestly untested, as this is not covered by FATE. libavformat/hls.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)