Re: [FFmpeg-devel] [PATCH] avformat/rtmpproto: Don't free AVOpt-strings manually, fix crash
Liu Steven 于2024年3月31日周日 10:16写道： > > > > > On Mar 30, 2024, at 12:21, Andreas Rheinhardt > > wrote: > > > > Andreas Rheinhardt: > Hi Andreas, > > >> Besides being redundant, freeing manually is actually harmful here, > >> as rtmp_close() may call gen_fcunpublish_stream() which dereferences > >> rt->playpath. > >> > >> Reported-by: Armin Hasitzka > >> Signed-off-by: Andreas Rheinhardt > >> --- > >> libavformat/rtmpproto.c | 3 --- > >> 1 file changed, 3 deletions(-) > >> > >> diff --git a/libavformat/rtmpproto.c b/libavformat/rtmpproto.c > >> index 4b01b67d28..b1d73b3d75 100644 > >> --- a/libavformat/rtmpproto.c > >> +++ b/libavformat/rtmpproto.c > >> @@ -2917,9 +2917,6 @@ reconnect: > >> return 0; > >> > >> fail: > >> -av_freep(&rt->playpath); > >> -av_freep(&rt->tcurl); > >> -av_freep(&rt->flashver); > >> av_dict_free(opts); > >> rtmp_close(s); > >> return ret; > > > > I am pinging this and explicitly cc'ing Steven Liu, whose commit > > 991cf95fdeebc3af added the av_freeps to be removed above. Steven, did > > you just feel that there was missing freeing code for the buffers above > > or was there an actually confirmed memleak (there shouldn't be)? > > Confirmed memleak, but it’s long time i cannot sure how to reproduce that, I > test rtmp those years use SRS. May be the memleak was reported by fuzz program, the test case can injection failed result make the workflow into the failed, The gen_fcunpublish_stream should after av_malloc rt->playpath, rt->tcurl, rt->flashver av_malloc, and there cloud get failed in the gen_connect. Steven Thanks ___ 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] avformat/rtmpproto: Don't free AVOpt-strings manually, fix crash
> On Mar 30, 2024, at 12:21, Andreas Rheinhardt > wrote: > > Andreas Rheinhardt: Hi Andreas, >> Besides being redundant, freeing manually is actually harmful here, >> as rtmp_close() may call gen_fcunpublish_stream() which dereferences >> rt->playpath. >> >> Reported-by: Armin Hasitzka >> Signed-off-by: Andreas Rheinhardt >> --- >> libavformat/rtmpproto.c | 3 --- >> 1 file changed, 3 deletions(-) >> >> diff --git a/libavformat/rtmpproto.c b/libavformat/rtmpproto.c >> index 4b01b67d28..b1d73b3d75 100644 >> --- a/libavformat/rtmpproto.c >> +++ b/libavformat/rtmpproto.c >> @@ -2917,9 +2917,6 @@ reconnect: >> return 0; >> >> fail: >> -av_freep(&rt->playpath); >> -av_freep(&rt->tcurl); >> -av_freep(&rt->flashver); >> av_dict_free(opts); >> rtmp_close(s); >> return ret; > > I am pinging this and explicitly cc'ing Steven Liu, whose commit > 991cf95fdeebc3af added the av_freeps to be removed above. Steven, did > you just feel that there was missing freeing code for the buffers above > or was there an actually confirmed memleak (there shouldn't be)? Confirmed memleak, but it’s long time i cannot sure how to reproduce that, I test rtmp those years use SRS. Thanks Steven ___ 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] avformat/rtmpproto: Don't free AVOpt-strings manually, fix crash
Andreas Rheinhardt: > Andreas Rheinhardt: >> Besides being redundant, freeing manually is actually harmful here, >> as rtmp_close() may call gen_fcunpublish_stream() which dereferences >> rt->playpath. >> >> Reported-by: Armin Hasitzka >> Signed-off-by: Andreas Rheinhardt >> --- >> libavformat/rtmpproto.c | 3 --- >> 1 file changed, 3 deletions(-) >> >> diff --git a/libavformat/rtmpproto.c b/libavformat/rtmpproto.c >> index 4b01b67d28..b1d73b3d75 100644 >> --- a/libavformat/rtmpproto.c >> +++ b/libavformat/rtmpproto.c >> @@ -2917,9 +2917,6 @@ reconnect: >> return 0; >> >> fail: >> -av_freep(&rt->playpath); >> -av_freep(&rt->tcurl); >> -av_freep(&rt->flashver); >> av_dict_free(opts); >> rtmp_close(s); >> return ret; > > I am pinging this and explicitly cc'ing Steven Liu, whose commit > 991cf95fdeebc3af added the av_freeps to be removed above. Steven, did > you just feel that there was missing freeing code for the buffers above > or was there an actually confirmed memleak (there shouldn't be)? > > - Andreas > Another ping explicitly cc'ing Steven Liu, this time with a more recent email. - 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".
Re: [FFmpeg-devel] [PATCH] avformat/rtmpproto: Don't free AVOpt-strings manually, fix crash
Andreas Rheinhardt: > Besides being redundant, freeing manually is actually harmful here, > as rtmp_close() may call gen_fcunpublish_stream() which dereferences > rt->playpath. > > Reported-by: Armin Hasitzka > Signed-off-by: Andreas Rheinhardt > --- > libavformat/rtmpproto.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/libavformat/rtmpproto.c b/libavformat/rtmpproto.c > index 4b01b67d28..b1d73b3d75 100644 > --- a/libavformat/rtmpproto.c > +++ b/libavformat/rtmpproto.c > @@ -2917,9 +2917,6 @@ reconnect: > return 0; > > fail: > -av_freep(&rt->playpath); > -av_freep(&rt->tcurl); > -av_freep(&rt->flashver); > av_dict_free(opts); > rtmp_close(s); > return ret; I am pinging this and explicitly cc'ing Steven Liu, whose commit 991cf95fdeebc3af added the av_freeps to be removed above. Steven, did you just feel that there was missing freeing code for the buffers above or was there an actually confirmed memleak (there shouldn't be)? - 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] avformat/rtmpproto: Don't free AVOpt-strings manually, fix crash
Besides being redundant, freeing manually is actually harmful here, as rtmp_close() may call gen_fcunpublish_stream() which dereferences rt->playpath. Reported-by: Armin Hasitzka Signed-off-by: Andreas Rheinhardt --- libavformat/rtmpproto.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/libavformat/rtmpproto.c b/libavformat/rtmpproto.c index 4b01b67d28..b1d73b3d75 100644 --- a/libavformat/rtmpproto.c +++ b/libavformat/rtmpproto.c @@ -2917,9 +2917,6 @@ reconnect: return 0; fail: -av_freep(&rt->playpath); -av_freep(&rt->tcurl); -av_freep(&rt->flashver); av_dict_free(opts); rtmp_close(s); return ret; -- 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-requ...@ffmpeg.org with subject "unsubscribe".
