* Re: [FFmpeg-devel] [PATCH 2/2] libavformat/takdec.c: Fix msan error. [not found] <20250407181039.353242-1-tfoucu@google.com> @ 2025-04-17 19:08 ` Thierry Foucu 2025-04-17 20:25 ` James Almer 0 siblings, 1 reply; 5+ messages in thread From: Thierry Foucu @ 2025-04-17 19:08 UTC (permalink / raw) To: ffmpeg-devel On Mon, Apr 7, 2025 at 11:10 AM <tfoucu@google.com> wrote: > From: Thierry Foucu <tfoucu@gmail.com> > > Make sure we are reading 16 bytes for the MD5 > --- > libavformat/takdec.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/libavformat/takdec.c b/libavformat/takdec.c > index 21fff3fcbf..61b9f001c4 100644 > --- a/libavformat/takdec.c > +++ b/libavformat/takdec.c > @@ -19,6 +19,7 @@ > * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA > 02110-1301 USA > */ > > +#include <libavutil/error.h> > #include "libavutil/crc.h" > #include "libavutil/mem.h" > > @@ -116,7 +117,9 @@ static int tak_read_header(AVFormatContext *s) > if (size != 19) > return AVERROR_INVALIDDATA; > ffio_init_checksum(pb, tak_check_crc, 0xCE04B7U); > - avio_read(pb, md5, 16); > + if (avio_read(pb, md5, 16) != 16) { > + return AVERROR(EIO); > + } > if (ffio_get_checksum(s->pb) != avio_rb24(pb)) { > av_log(s, AV_LOG_ERROR, "MD5 metadata block CRC > error.\n"); > if (s->error_recognition & AV_EF_EXPLODE) > -- > 2.49.0.504.g3bcea36a83-goog > > Ping? _______________________________________________ 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] 5+ messages in thread
* Re: [FFmpeg-devel] [PATCH 2/2] libavformat/takdec.c: Fix msan error. 2025-04-17 19:08 ` [FFmpeg-devel] [PATCH 2/2] libavformat/takdec.c: Fix msan error Thierry Foucu @ 2025-04-17 20:25 ` James Almer 2025-04-21 16:56 ` Thierry Foucu 0 siblings, 1 reply; 5+ messages in thread From: James Almer @ 2025-04-17 20:25 UTC (permalink / raw) To: ffmpeg-devel [-- Attachment #1.1.1: Type: text/plain, Size: 1811 bytes --] On 4/17/2025 4:08 PM, Thierry Foucu wrote: > On Mon, Apr 7, 2025 at 11:10 AM <tfoucu@google.com> wrote: > >> From: Thierry Foucu <tfoucu@gmail.com> >> >> Make sure we are reading 16 bytes for the MD5 >> --- >> libavformat/takdec.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/libavformat/takdec.c b/libavformat/takdec.c >> index 21fff3fcbf..61b9f001c4 100644 >> --- a/libavformat/takdec.c >> +++ b/libavformat/takdec.c >> @@ -19,6 +19,7 @@ >> * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA >> 02110-1301 USA >> */ >> >> +#include <libavutil/error.h> >> #include "libavutil/crc.h" >> #include "libavutil/mem.h" >> >> @@ -116,7 +117,9 @@ static int tak_read_header(AVFormatContext *s) >> if (size != 19) >> return AVERROR_INVALIDDATA; >> ffio_init_checksum(pb, tak_check_crc, 0xCE04B7U); >> - avio_read(pb, md5, 16); >> + if (avio_read(pb, md5, 16) != 16) { >> + return AVERROR(EIO); Should be AVERROR_INVALIDDATA imo, since i means the file is just truncated. It wasn't an error in the protocol reading data. Will amend with that change and push. Thanks. >> + } >> if (ffio_get_checksum(s->pb) != avio_rb24(pb)) { >> av_log(s, AV_LOG_ERROR, "MD5 metadata block CRC >> error.\n"); >> if (s->error_recognition & AV_EF_EXPLODE) >> -- >> 2.49.0.504.g3bcea36a83-goog >> >> > Ping? > _______________________________________________ > 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". [-- Attachment #1.2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 495 bytes --] [-- Attachment #2: Type: text/plain, Size: 251 bytes --] _______________________________________________ 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] 5+ messages in thread
* Re: [FFmpeg-devel] [PATCH 2/2] libavformat/takdec.c: Fix msan error. 2025-04-17 20:25 ` James Almer @ 2025-04-21 16:56 ` Thierry Foucu 2025-04-21 17:00 ` Nicolas George 0 siblings, 1 reply; 5+ messages in thread From: Thierry Foucu @ 2025-04-21 16:56 UTC (permalink / raw) To: FFmpeg development discussions and patches Thanks James On Thu, Apr 17, 2025 at 1:25 PM James Almer <jamrial@gmail.com> wrote: > On 4/17/2025 4:08 PM, Thierry Foucu wrote: > > On Mon, Apr 7, 2025 at 11:10 AM <tfoucu@google.com> wrote: > > > >> From: Thierry Foucu <tfoucu@gmail.com> > >> > >> Make sure we are reading 16 bytes for the MD5 > >> --- > >> libavformat/takdec.c | 5 ++++- > >> 1 file changed, 4 insertions(+), 1 deletion(-) > >> > >> diff --git a/libavformat/takdec.c b/libavformat/takdec.c > >> index 21fff3fcbf..61b9f001c4 100644 > >> --- a/libavformat/takdec.c > >> +++ b/libavformat/takdec.c > >> @@ -19,6 +19,7 @@ > >> * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA > >> 02110-1301 USA > >> */ > >> > >> +#include <libavutil/error.h> > >> #include "libavutil/crc.h" > >> #include "libavutil/mem.h" > >> > >> @@ -116,7 +117,9 @@ static int tak_read_header(AVFormatContext *s) > >> if (size != 19) > >> return AVERROR_INVALIDDATA; > >> ffio_init_checksum(pb, tak_check_crc, 0xCE04B7U); > >> - avio_read(pb, md5, 16); > >> + if (avio_read(pb, md5, 16) != 16) { > >> + return AVERROR(EIO); > > Should be AVERROR_INVALIDDATA imo, since i means the file is just > truncated. It wasn't an error in the protocol reading data. > > Regarding the error message, I was trying to be consistent with the other avio_read error, like here: https://github.com/FFmpeg/FFmpeg/blob/master/libavformat/takdec.c#L99 > Will amend with that change and push. Thanks. > > >> + } > >> if (ffio_get_checksum(s->pb) != avio_rb24(pb)) { > >> av_log(s, AV_LOG_ERROR, "MD5 metadata block CRC > >> error.\n"); > >> if (s->error_recognition & AV_EF_EXPLODE) > >> -- > >> 2.49.0.504.g3bcea36a83-goog > >> > >> > > Ping? > > _______________________________________________ > > 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". > > _______________________________________________ > 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". > -- Thierry Foucu _______________________________________________ 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] 5+ messages in thread
* Re: [FFmpeg-devel] [PATCH 2/2] libavformat/takdec.c: Fix msan error. 2025-04-21 16:56 ` Thierry Foucu @ 2025-04-21 17:00 ` Nicolas George 2025-04-21 17:40 ` Thierry Foucu 0 siblings, 1 reply; 5+ messages in thread From: Nicolas George @ 2025-04-21 17:00 UTC (permalink / raw) To: FFmpeg development discussions and patches Thierry Foucu (HE12025-04-21): > > >> - avio_read(pb, md5, 16); > > >> + if (avio_read(pb, md5, 16) != 16) { > > >> + return AVERROR(EIO); > > Should be AVERROR_INVALIDDATA imo, since i means the file is just > > truncated. It wasn't an error in the protocol reading data. > Regarding the error message, I was trying to be consistent with the other > avio_read error, like here: > https://github.com/FFmpeg/FFmpeg/blob/master/libavformat/takdec.c#L99 Both codes are wrong: if avio_read() returns an error, it must be returned as is, not a new error invented. And if avio_read() returns less than requested, then I agree with James, INVALID_DATA is more correct than EIO. In both places. Regards, -- Nicolas George _______________________________________________ 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] 5+ messages in thread
* Re: [FFmpeg-devel] [PATCH 2/2] libavformat/takdec.c: Fix msan error. 2025-04-21 17:00 ` Nicolas George @ 2025-04-21 17:40 ` Thierry Foucu 0 siblings, 0 replies; 5+ messages in thread From: Thierry Foucu @ 2025-04-21 17:40 UTC (permalink / raw) To: FFmpeg development discussions and patches On Mon, Apr 21, 2025 at 10:01 AM Nicolas George <george@nsup.org> wrote: > Thierry Foucu (HE12025-04-21): > > > >> - avio_read(pb, md5, 16); > > > >> + if (avio_read(pb, md5, 16) != 16) { > > > >> + return AVERROR(EIO); > > > Should be AVERROR_INVALIDDATA imo, since i means the file is just > > > truncated. It wasn't an error in the protocol reading data. > > Regarding the error message, I was trying to be consistent with the other > > avio_read error, like here: > > https://github.com/FFmpeg/FFmpeg/blob/master/libavformat/takdec.c#L99 > > Both codes are wrong: if avio_read() returns an error, it must be > returned as is, not a new error invented. > > And if avio_read() returns less than requested, then I agree with James, > INVALID_DATA is more correct than EIO. In both places. > Thanks. This is good to know.. > > Regards, > > -- > Nicolas George > _______________________________________________ > 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". > -- Thierry Foucu _______________________________________________ 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] 5+ messages in thread
end of thread, other threads:[~2025-04-21 17:41 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20250407181039.353242-1-tfoucu@google.com>
2025-04-17 19:08 ` [FFmpeg-devel] [PATCH 2/2] libavformat/takdec.c: Fix msan error Thierry Foucu
2025-04-17 20:25 ` James Almer
2025-04-21 16:56 ` Thierry Foucu
2025-04-21 17:00 ` Nicolas George
2025-04-21 17:40 ` Thierry Foucu
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