| Message ID | 1551842307-3607-3-git-send-email-mypopydev@gmail.com |
|---|---|
| State | New |
| Headers | show |
2019-03-06 4:18 GMT+01:00, Jun Zhao <mypopydev@gmail.com>: > From: Jun Zhao <barryjzhao@tencent.com> > > accumulation of 8-bits uint_8 (uint8_t *src) into 32-bits (uint32_t *ii) > data type, it will have a risk of an integral value becoming larger than > the 32-bits integer capacity and resulting in an integer overflow. For > this risk, add a checking with warning message. > > Signed-off-by: Jun Zhao <barryjzhao@tencent.com> > --- > libavfilter/vf_nlmeans.c | 7 +++++++ > 1 files changed, 7 insertions(+), 0 deletions(-) > > diff --git a/libavfilter/vf_nlmeans.c b/libavfilter/vf_nlmeans.c > index dcb5a03..31c8304 100644 > --- a/libavfilter/vf_nlmeans.c > +++ b/libavfilter/vf_nlmeans.c > @@ -236,6 +236,13 @@ static void compute_ssd_integral_image(const > NLMeansDSPContext *dsp, > // adjusted end x position of the safe area after width of the safe > area gets aligned > const int endx_safe = startx_safe + safe_pw; > > + // accumulation of 8-bits uint_8 (uint8_t *src) into 32-bits (uint32_t > *ii) > + // data type, it will have a risk of an integral value becoming larger > than > + // the 32-bits integer capacity and resulting in an integer overflow. > + if ((w * h * UINT8_MAX) > UINT32_MAX) I don't think UINT8_MAX increases readability and I suspect this should contain "UINT32_MAX / (w*h)" or similar on one side. > + av_log(NULL, AV_LOG_WARNING, > + "image (%d x %d) integral value maybe overflow.\n", w ,h); may overflow? Carl Eugen
On Wed, Mar 6, 2019 at 3:55 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > 2019-03-06 4:18 GMT+01:00, Jun Zhao <mypopydev@gmail.com>: > > From: Jun Zhao <barryjzhao@tencent.com> > > > > accumulation of 8-bits uint_8 (uint8_t *src) into 32-bits (uint32_t *ii) > > data type, it will have a risk of an integral value becoming larger than > > the 32-bits integer capacity and resulting in an integer overflow. For > > this risk, add a checking with warning message. > > > > Signed-off-by: Jun Zhao <barryjzhao@tencent.com> > > --- > > libavfilter/vf_nlmeans.c | 7 +++++++ > > 1 files changed, 7 insertions(+), 0 deletions(-) > > > > diff --git a/libavfilter/vf_nlmeans.c b/libavfilter/vf_nlmeans.c > > index dcb5a03..31c8304 100644 > > --- a/libavfilter/vf_nlmeans.c > > +++ b/libavfilter/vf_nlmeans.c > > @@ -236,6 +236,13 @@ static void compute_ssd_integral_image(const > > NLMeansDSPContext *dsp, > > // adjusted end x position of the safe area after width of the safe > > area gets aligned > > const int endx_safe = startx_safe + safe_pw; > > > > + // accumulation of 8-bits uint_8 (uint8_t *src) into 32-bits (uint32_t > > *ii) > > + // data type, it will have a risk of an integral value becoming larger > > than > > + // the 32-bits integer capacity and resulting in an integer overflow. > > + if ((w * h * UINT8_MAX) > UINT32_MAX) > > I don't think UINT8_MAX increases readability and I suspect > this should contain "UINT32_MAX / (w*h)" or similar on > one side. > You means like: UINT32_MAX/w < (UINT8_MAX * h) ? > > + av_log(NULL, AV_LOG_WARNING, > > + "image (%d x %d) integral value maybe overflow.\n", w ,h); > > may overflow? Will update the warning message as the suggestion.Tks
2019-03-06 12:31 GMT+01:00, mypopy@gmail.com <mypopy@gmail.com>: > On Wed, Mar 6, 2019 at 3:55 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: >> >> 2019-03-06 4:18 GMT+01:00, Jun Zhao <mypopydev@gmail.com>: >> > + // accumulation of 8-bits uint_8 (uint8_t *src) into 32-bits >> > (uint32_t *ii) >> > + // data type, it will have a risk of an integral value becoming >> > larger than >> > + // the 32-bits integer capacity and resulting in an integer >> > overflow. >> > + if ((w * h * UINT8_MAX) > UINT32_MAX) >> >> I don't think UINT8_MAX increases readability and I suspect >> this should contain "UINT32_MAX / (w*h)" or similar on >> one side. >> > You means like: UINT32_MAX/w < (UINT8_MAX * h) ? Actually: (UINT32_MAX / w < 255 * h) (But that may only be me) Carl Eugen
On Wed, Mar 6, 2019 at 9:15 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > 2019-03-06 12:31 GMT+01:00, mypopy@gmail.com <mypopy@gmail.com>: > > On Wed, Mar 6, 2019 at 3:55 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > >> > >> 2019-03-06 4:18 GMT+01:00, Jun Zhao <mypopydev@gmail.com>: > > >> > + // accumulation of 8-bits uint_8 (uint8_t *src) into 32-bits > >> > (uint32_t *ii) > >> > + // data type, it will have a risk of an integral value becoming > >> > larger than > >> > + // the 32-bits integer capacity and resulting in an integer > >> > overflow. > >> > + if ((w * h * UINT8_MAX) > UINT32_MAX) > >> > >> I don't think UINT8_MAX increases readability and I suspect > >> this should contain "UINT32_MAX / (w*h)" or similar on > >> one side. > >> > > You means like: UINT32_MAX/w < (UINT8_MAX * h) ? > > Actually: (UINT32_MAX / w < 255 * h) > (But that may only be me) > Update V3 patch for this check in https://patchwork.ffmpeg.org/patch/12230/, Tks
diff --git a/libavfilter/vf_nlmeans.c b/libavfilter/vf_nlmeans.c index dcb5a03..31c8304 100644 --- a/libavfilter/vf_nlmeans.c +++ b/libavfilter/vf_nlmeans.c @@ -236,6 +236,13 @@ static void compute_ssd_integral_image(const NLMeansDSPContext *dsp, // adjusted end x position of the safe area after width of the safe area gets aligned const int endx_safe = startx_safe + safe_pw; + // accumulation of 8-bits uint_8 (uint8_t *src) into 32-bits (uint32_t *ii) + // data type, it will have a risk of an integral value becoming larger than + // the 32-bits integer capacity and resulting in an integer overflow. + if ((w * h * UINT8_MAX) > UINT32_MAX) + av_log(NULL, AV_LOG_WARNING, + "image (%d x %d) integral value maybe overflow.\n", w ,h); + // top part where only one of s1 and s2 is still readable, or none at all compute_unsafe_ssd_integral_image(ii, ii_linesize_32, 0, 0,