Re: [FFmpeg-devel] [PATCH 1/3] avcodec/alsdec: Limit maximum channels to 64
On Tue, Aug 20, 2019 at 09:12:36AM +0200, Thilo Borgmann wrote: > Am 19.08.19 um 23:22 schrieb Michael Niedermayer: > > On Mon, Aug 19, 2019 at 05:09:37PM +0200, Thilo Borgmann wrote: > >> Am 19.08.19 um 14:27 schrieb Michael Niedermayer: > >>> On Mon, Aug 19, 2019 at 07:41:43AM +0200, Thilo Borgmann wrote: > Am 19.08.19 um 01:30 schrieb Michael Niedermayer: > > There seems to be no limit in the specification and upto 64k could be > > stored > > 64 is chooses as limit as thats also used for AAC and is what a int64 > > mask > > can handle > > > > An alternative to this patch would be a max_channels variable > > There's a conformance file containing 512 channels, that should be the > default max value. > >>> > >>> will apply with that value later > >> > >> Decoding of that is already stopped via FF_SANE_NB_CHANNELS in > >> lavc/internal.h. > >> That is currently set to 256U. I guess pushing that to 512U might already > >> be enough without any change to single decoders? > > > > the problem of out of memory is that the als decoder allocates some pretty > > large things per channel. A check on channels needs to happen before > > this. > > The existing checks didnt achieve that. > > > > I can of course post a patchset that uses FF_SANE_NB_CHANNELS in als and > > bump it to the next number if people prefer this ? > > > > Note though that increasing FF_SANE_NB_CHANNELS may also increase timeouts > > in other decoders. > > I see the problem. > However, right now alsdec is fine with any number of channels and the CLI > prevents decoding anything above FF_SANE_NB_CHANNELS (checks are in > lavc/decoding.c and lavc/utils.c; this would also stop any FATE test on 512 > channels file). > > So having alsdec check for a value higher than FF_SANE_NB_CHANNELS (like 512) > would have no effect for CLI users, right? yes, just wanted to make sure the side effect of a 512 FF_SANE_NB_CHANNELS is understood will post the patches thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The real ebay dictionary, page 2 "100% positive feedback" - "All either got their money back or didnt complain" "Best seller ever, very honest" - "Seller refunded buyer after failed scam" signature.asc Description: PGP signature ___ 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 1/3] avcodec/alsdec: Limit maximum channels to 64
Am 19.08.19 um 23:22 schrieb Michael Niedermayer: > On Mon, Aug 19, 2019 at 05:09:37PM +0200, Thilo Borgmann wrote: >> Am 19.08.19 um 14:27 schrieb Michael Niedermayer: >>> On Mon, Aug 19, 2019 at 07:41:43AM +0200, Thilo Borgmann wrote: Am 19.08.19 um 01:30 schrieb Michael Niedermayer: > There seems to be no limit in the specification and upto 64k could be > stored > 64 is chooses as limit as thats also used for AAC and is what a int64 mask > can handle > > An alternative to this patch would be a max_channels variable There's a conformance file containing 512 channels, that should be the default max value. >>> >>> will apply with that value later >> >> Decoding of that is already stopped via FF_SANE_NB_CHANNELS in >> lavc/internal.h. >> That is currently set to 256U. I guess pushing that to 512U might already be >> enough without any change to single decoders? > > the problem of out of memory is that the als decoder allocates some pretty > large things per channel. A check on channels needs to happen before > this. > The existing checks didnt achieve that. > > I can of course post a patchset that uses FF_SANE_NB_CHANNELS in als and > bump it to the next number if people prefer this ? > > Note though that increasing FF_SANE_NB_CHANNELS may also increase timeouts > in other decoders. I see the problem. However, right now alsdec is fine with any number of channels and the CLI prevents decoding anything above FF_SANE_NB_CHANNELS (checks are in lavc/decoding.c and lavc/utils.c; this would also stop any FATE test on 512 channels file). So having alsdec check for a value higher than FF_SANE_NB_CHANNELS (like 512) would have no effect for CLI users, right? -Thilo >> Once fixed, I will add a FATE test for the 512 channel conformance file. > > thx > >> >> -Thilo >> ___ >> 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". > ___ 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 1/3] avcodec/alsdec: Limit maximum channels to 64
On Mon, Aug 19, 2019 at 05:09:37PM +0200, Thilo Borgmann wrote: > Am 19.08.19 um 14:27 schrieb Michael Niedermayer: > > On Mon, Aug 19, 2019 at 07:41:43AM +0200, Thilo Borgmann wrote: > >> Am 19.08.19 um 01:30 schrieb Michael Niedermayer: > >>> There seems to be no limit in the specification and upto 64k could be > >>> stored > >>> 64 is chooses as limit as thats also used for AAC and is what a int64 mask > >>> can handle > >>> > >>> An alternative to this patch would be a max_channels variable > >> > >> There's a conformance file containing 512 channels, that should be the > >> default max value. > > > > will apply with that value later > > Decoding of that is already stopped via FF_SANE_NB_CHANNELS in > lavc/internal.h. > That is currently set to 256U. I guess pushing that to 512U might already be > enough without any change to single decoders? the problem of out of memory is that the als decoder allocates some pretty large things per channel. A check on channels needs to happen before this. The existing checks didnt achieve that. I can of course post a patchset that uses FF_SANE_NB_CHANNELS in als and bump it to the next number if people prefer this ? Note though that increasing FF_SANE_NB_CHANNELS may also increase timeouts in other decoders. > > Once fixed, I will add a FATE test for the 512 channel conformance file. thx > > -Thilo > ___ > 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". -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB No human being will ever know the Truth, for even if they happen to say it by chance, they would not even known they had done so. -- Xenophanes signature.asc Description: PGP signature ___ 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 1/3] avcodec/alsdec: Limit maximum channels to 64
Am 19.08.19 um 14:27 schrieb Michael Niedermayer: > On Mon, Aug 19, 2019 at 07:41:43AM +0200, Thilo Borgmann wrote: >> Am 19.08.19 um 01:30 schrieb Michael Niedermayer: >>> There seems to be no limit in the specification and upto 64k could be stored >>> 64 is chooses as limit as thats also used for AAC and is what a int64 mask >>> can handle >>> >>> An alternative to this patch would be a max_channels variable >> >> There's a conformance file containing 512 channels, that should be the >> default max value. > > will apply with that value later Decoding of that is already stopped via FF_SANE_NB_CHANNELS in lavc/internal.h. That is currently set to 256U. I guess pushing that to 512U might already be enough without any change to single decoders? Once fixed, I will add a FATE test for the 512 channel conformance file. -Thilo ___ 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 1/3] avcodec/alsdec: Limit maximum channels to 64
On Mon, Aug 19, 2019 at 07:41:43AM +0200, Thilo Borgmann wrote: > Am 19.08.19 um 01:30 schrieb Michael Niedermayer: > > There seems to be no limit in the specification and upto 64k could be stored > > 64 is chooses as limit as thats also used for AAC and is what a int64 mask > > can handle > > > > An alternative to this patch would be a max_channels variable > > There's a conformance file containing 512 channels, that should be the > default max value. can you add a fate test with that sample ? thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Freedom in capitalist society always remains about the same as it was in ancient Greek republics: Freedom for slave owners. -- Vladimir Lenin signature.asc Description: PGP signature ___ 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 1/3] avcodec/alsdec: Limit maximum channels to 64
On Mon, Aug 19, 2019 at 07:41:43AM +0200, Thilo Borgmann wrote: > Am 19.08.19 um 01:30 schrieb Michael Niedermayer: > > There seems to be no limit in the specification and upto 64k could be stored > > 64 is chooses as limit as thats also used for AAC and is what a int64 mask > > can handle > > > > An alternative to this patch would be a max_channels variable > > There's a conformance file containing 512 channels, that should be the > default max value. will apply with that value later thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB In fact, the RIAA has been known to suggest that students drop out of college or go to community college in order to be able to afford settlements. -- The RIAA signature.asc Description: PGP signature ___ 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 1/3] avcodec/alsdec: Limit maximum channels to 64
Am 19.08.19 um 01:30 schrieb Michael Niedermayer: > There seems to be no limit in the specification and upto 64k could be stored > 64 is chooses as limit as thats also used for AAC and is what a int64 mask > can handle > > An alternative to this patch would be a max_channels variable There's a conformance file containing 512 channels, that should be the default max value. -Thilo ___ 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 1/3] avcodec/alsdec: Limit maximum channels to 64
There seems to be no limit in the specification and upto 64k could be stored
64 is chooses as limit as thats also used for AAC and is what a int64 mask
can handle
An alternative to this patch would be a max_channels variable
Fixes: OOM
Fixes:
16200/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_ALS_fuzzer-5764788793114624
Found-by: continuous fuzzing process
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer
---
libavcodec/alsdec.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/libavcodec/alsdec.c b/libavcodec/alsdec.c
index 26c496c769..425cf73be9 100644
--- a/libavcodec/alsdec.c
+++ b/libavcodec/alsdec.c
@@ -43,6 +43,8 @@
#include
+#define MAX_CHANNELS 64
+
/** Rice parameters and corresponding index offsets for decoding the
* indices of scaled PARCOR values. The table chosen is set globally
* by the encoder and stored in ALSSpecificConfig.
@@ -348,6 +350,11 @@ static av_cold int read_specific_config(ALSDecContext *ctx)
if (als_id != MKBETAG('A','L','S','\0'))
return AVERROR_INVALIDDATA;
+if (avctx->channels > MAX_CHANNELS) {
+avpriv_request_sample(avctx, "Huge number of channels\n");
+return AVERROR_PATCHWELCOME;
+}
+
ctx->cur_frame_length = sconf->frame_length;
// read channel config
--
2.22.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".
