[FFmpeg-devel] [PATCH V3] lavfi/nlmeans: Checking number precision when computing integral images

[Michael Niedermayer]

On Thu, Mar 07, 2019 at 09:18:42PM +0100, Michael Niedermayer wrote:
> On Thu, Mar 07, 2019 at 09:45:12AM +0800, Jun Zhao wrote:
> > From: Jun Zhao <barryjzhao at 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 at 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..9876aae 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 ((UINT32_MAX / (uint64_t)w) < (255 * (uint64_t)h))
> > +        av_log(NULL, AV_LOG_WARNING,
> > +               "image (%d x %d) integral value may overflow.\n", w ,h);
> 
> Printing a warning is not an adequate response for a integer overflow.
> Such thing is undefined behavior (in case signed of signed int) and must
> not occur.

And if no signed ints are involved, while this is then not undefined it still
gives the wrong result. Thats a bug, the bug should be fixed not a warning
be printed that the bug might be triggered

Thanks

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

There will always be a question for which you do not know the correct answer.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20190307/5dc03a0e/attachment.sig>