Re: [FFmpeg-devel] [PATCH] avutil/imgutils: don't add offsets to NULL pointers
On 5/4/2021 5:50 PM, James Almer wrote: On 5/4/2021 5:13 PM, Andreas Rheinhardt wrote: James Almer: Signed-off-by: James Almer --- libavutil/imgutils.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavutil/imgutils.c b/libavutil/imgutils.c index 53faad889a..aaee0dfb7a 100644 --- a/libavutil/imgutils.c +++ b/libavutil/imgutils.c @@ -166,7 +166,7 @@ int av_image_fill_pointers(uint8_t *data[4], enum AVPixelFormat pix_fmt, int hei } data[0] = ptr; - for (i = 1; i < 4 && sizes[i]; i++) + for (i = 1; i < 4 && data[i - 1] && sizes[i]; i++) data[i] = data[i - 1] + sizes[i - 1]; return ret; I see two ways to make this a NULL + offset: First, if ptr == NULL; and second if data[i - 1] + sizes[i - 1] no longer fits into the allocated buffer and happens to yield NULL (very unlikely, but possible) in which case data[i] + sizes[i] would be NULL + offset. In the second case, the first addition is already undefined behaviour against which we cannot guard at all: We don't know the size of the buffer. The only thing we can guard against is ptr being NULL; we can even error out in this scenario, but I don't know how disruptive that would be. That'd be an undesirable breakage, yes. Aside from filling data[], the function also returns the size of the buffer that should be allocated, so that functionality should remain even when ptr == NULL. Notice that in C the result of pointer + offset can never be NULL, so a compiler could optimize the check for data[i - 1] to just a check for ptr. If you say there's no warranty that an scenario where data[i-1] + size[i-1] == NULL will break the for loop in the next iteration, and no way to guard against it at all, then we can just return right before attempting to set data[] when ptr == NULL, and at least simplify that scenario. Made that change and pushed. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avutil/imgutils: don't add offsets to NULL pointers
On 5/4/2021 5:13 PM, Andreas Rheinhardt wrote: James Almer: Signed-off-by: James Almer --- libavutil/imgutils.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavutil/imgutils.c b/libavutil/imgutils.c index 53faad889a..aaee0dfb7a 100644 --- a/libavutil/imgutils.c +++ b/libavutil/imgutils.c @@ -166,7 +166,7 @@ int av_image_fill_pointers(uint8_t *data[4], enum AVPixelFormat pix_fmt, int hei } data[0] = ptr; -for (i = 1; i < 4 && sizes[i]; i++) +for (i = 1; i < 4 && data[i - 1] && sizes[i]; i++) data[i] = data[i - 1] + sizes[i - 1]; return ret; I see two ways to make this a NULL + offset: First, if ptr == NULL; and second if data[i - 1] + sizes[i - 1] no longer fits into the allocated buffer and happens to yield NULL (very unlikely, but possible) in which case data[i] + sizes[i] would be NULL + offset. In the second case, the first addition is already undefined behaviour against which we cannot guard at all: We don't know the size of the buffer. The only thing we can guard against is ptr being NULL; we can even error out in this scenario, but I don't know how disruptive that would be. That'd be an undesirable breakage, yes. Aside from filling data[], the function also returns the size of the buffer that should be allocated, so that functionality should remain even when ptr == NULL. Notice that in C the result of pointer + offset can never be NULL, so a compiler could optimize the check for data[i - 1] to just a check for ptr. If you say there's no warranty that an scenario where data[i-1] + size[i-1] == NULL will break the for loop in the next iteration, and no way to guard against it at all, then we can just return right before attempting to set data[] when ptr == NULL, and at least simplify that scenario. - 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-requ...@ffmpeg.org with subject "unsubscribe". ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avutil/imgutils: don't add offsets to NULL pointers
James Almer: > Signed-off-by: James Almer > --- > libavutil/imgutils.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavutil/imgutils.c b/libavutil/imgutils.c > index 53faad889a..aaee0dfb7a 100644 > --- a/libavutil/imgutils.c > +++ b/libavutil/imgutils.c > @@ -166,7 +166,7 @@ int av_image_fill_pointers(uint8_t *data[4], enum > AVPixelFormat pix_fmt, int hei > } > > data[0] = ptr; > -for (i = 1; i < 4 && sizes[i]; i++) > +for (i = 1; i < 4 && data[i - 1] && sizes[i]; i++) > data[i] = data[i - 1] + sizes[i - 1]; > > return ret; > I see two ways to make this a NULL + offset: First, if ptr == NULL; and second if data[i - 1] + sizes[i - 1] no longer fits into the allocated buffer and happens to yield NULL (very unlikely, but possible) in which case data[i] + sizes[i] would be NULL + offset. In the second case, the first addition is already undefined behaviour against which we cannot guard at all: We don't know the size of the buffer. The only thing we can guard against is ptr being NULL; we can even error out in this scenario, but I don't know how disruptive that would be. Notice that in C the result of pointer + offset can never be NULL, so a compiler could optimize the check for data[i - 1] to just a check for ptr. - 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-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] avutil/imgutils: don't add offsets to NULL pointers
Signed-off-by: James Almer --- libavutil/imgutils.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavutil/imgutils.c b/libavutil/imgutils.c index 53faad889a..aaee0dfb7a 100644 --- a/libavutil/imgutils.c +++ b/libavutil/imgutils.c @@ -166,7 +166,7 @@ int av_image_fill_pointers(uint8_t *data[4], enum AVPixelFormat pix_fmt, int hei } data[0] = ptr; -for (i = 1; i < 4 && sizes[i]; i++) +for (i = 1; i < 4 && data[i - 1] && sizes[i]; i++) data[i] = data[i - 1] + sizes[i - 1]; return ret; -- 2.31.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-requ...@ffmpeg.org with subject "unsubscribe".
