Issue 254 in webp: scaled decodes seem to use uninitialized memory

[we...@googlecode.com]

Status: New
Owner: ----
Labels: Type-Defect Priority-Medium

New issue 254 by scro...@google.com: scaled decodes seem to use
uninitialized memory
https://code.google.com/p/webp/issues/detail?id=254

In Skia (skia.org), we are using libwebp version 0.4.3.

We run tests that decode an image (at various scales) and then draw the
result. Then we compare the results over time to see if we've made a change
that generated a different result.

When we decode a webp image scaled, we see results that are different on
almost every run. Most of the image looks the same, but there are some
pixels along the right edge (that seem to be somewhat evenly spaced) that
look slightly different.

We also run a valgrind bot which complains of using uninitialized memory,
which I suspect is related. (An example is here:
https://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-Valgrind/builds/295/steps/dm/logs/stdio.)

I have attached a sample image along with a couple of results (scaled to
0.625) and the difference between these two results.



Attachments:
5.sm.webp 10.9 KB
oneRun.png 58.0 KB
anotherRun.png 58.0 KB
difference.png 1.1 KB

--
You received this message because this project is configured to send all
issue notifications to this address.
You may adjust your notification preferences at:
https://code.google.com/hosting/settings

[we...@googlecode.com]

Comment #1 on issue 254 by jz...@google.com: scaled decodes seem to use
uninitialized memory
https://code.google.com/p/webp/issues/detail?id=254

Thanks for the report. I imagine we'll be able to reproduce the report with
cwebp. Just for definiteness can you give the dimensions you were scaling
to rather than just the factor?

[we...@googlecode.com]

Comment #2 on issue 254 by scro...@google.com: scaled decodes seem to use
uninitialized memory
https://code.google.com/p/webp/issues/detail?id=254

This particular image was scaled to 200 x 147 in this instance, although we
found similar behavior at several scales.

[we...@googlecode.com]

Comment #3 on issue 254 by jz...@google.com: scaled decodes seem to use
uninitialized memory
https://code.google.com/p/webp/issues/detail?id=254

I can reproduce this doing an upscale with 0.4.3 and the current tip of
tree. A good way to achieve a difference is with different values of
--malloc-fill:

$ valgrind \
--track-origins=yes \
--malloc-fill=0xcc \
./examples/dwebp \
-ppm -scale 200 147 \
5.sm.webp -o out.ppm

I'll have a closer look when I get a chance.

[we...@googlecode.com]

Updates:
Status: Accepted
Owner: pascal.m...@gmail.com

Comment #4 on issue 254 by pascal.m...@gmail.com: scaled decodes seem to

use uninitialized memory
https://code.google.com/p/webp/issues/detail?id=254

there's apparently a buffer-read overflow happening at
src/utils/rescaler.c:51, where 'x_in' can become equal to 'wrk->src_width'
(shouldn't happen). Investigating and fixing...

[we...@googlecode.com]

Comment #5 on issue 254 by jz...@google.com: scaled decodes seem to use
uninitialized memory
https://code.google.com/p/webp/issues/detail?id=254

Right one could clamp or perhaps bias 'accum' given the current precision.
Increasing the precision would likely give a more accurate calculation of
this coordinate.

[we...@googlecode.com]

Comment #6 on issue 254 by pascal.m...@gmail.com: scaled decodes seem to

use uninitialized memory
https://code.google.com/p/webp/issues/detail?id=254

patch https://chromium-review.googlesource.com/#/c/290560/ should address
the issue and is out for review.

[we...@googlecode.com]

Updates:
Status: Fixed

Comment #7 on issue 254 by s...@google.com: scaled decodes seem to use
uninitialized memory
https://code.google.com/p/webp/issues/detail?id=254

the patches have been submitted.
It should address the issue. Can you cross-check please?

Thanks for the report!

[we...@googlecode.com]

Comment #8 on issue 254 by scro...@google.com: scaled decodes seem to use
uninitialized memory
https://code.google.com/p/webp/issues/detail?id=254

Thanks for the fix!

I'll make sure it works in Skia. The Skia bug is tracked in
https://code.google.com/p/skia/issues/detail?id=4038 - you can follow along
there, but I'll report back here if it still shows problems.

[we...@googlecode.com]

Comment #9 on issue 254 by scro...@google.com: scaled decodes seem to use
uninitialized memory
https://code.google.com/p/webp/issues/detail?id=254

From the comment in https://codereview.chromium.org/1280073002/#msg13 it
sounds like this is still not fixed for odd heights?

[we...@googlecode.com]

Comment #10 on issue 254 by jz...@google.com: scaled decodes seem to use
uninitialized memory
https://code.google.com/p/webp/issues/detail?id=254

Correct, we'll get a fix in for that soon. The problem there is not an out
of bounds read, but an incorrect final row.

[we...@googlecode.com]

Comment #11 on issue 254 by scro...@google.com: scaled decodes seem to use
uninitialized memory
https://code.google.com/p/webp/issues/detail?id=254

I just realized this is still marked as fixed. Did you fix the problem with
odd heights? Or is it tracked in another bug?

[we...@googlecode.com]

Updates:
Status: Accepted

Comment #12 on issue 254 by jz...@google.com: scaled decodes seem to use
uninitialized memory
https://code.google.com/p/webp/issues/detail?id=254

Reopening to track the remaining final row issue, fix is still pending.

[we...@googlecode.com]

Updates:
Status: Fixed

Comment #13 on issue 254 by jz...@google.com: scaled decodes seem to use
uninitialized memory
https://code.google.com/p/webp/issues/detail?id=254

The fix has been merged, all issues in this bug should be resolved.

https://chromium-review.googlesource.com/#/c/299840/
5ff0079 fix rescaler vertical interpolation

[we...@googlecode.com]

Comment #14 on issue 254 by pascal.m...@gmail.com: scaled decodes seem to

use uninitialized memory
https://code.google.com/p/webp/issues/detail?id=254

follow-up note: some notable speed improvement to the rescaler code
submitted recently:

NEON:
https://chromium.googlesource.com/webm/libwebp/+/b4e731cd93f9e86e9ea8cde7a0c0aa54ca0d777d
SSE2:
https://chromium.googlesource.com/webm/libwebp/+/76a7dc39e57ae192f521779c81c6e0b8c5bd4e85https://chromium.googlesource.com/webm%2Flibwebp/+/932fd4df61c90b52e69c391b13b32ba22f8149f3