After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 754387 - test-suite failure: ERROR: cve-2015-4491 - missing test plan
test-suite failure: ERROR: cve-2015-4491 - missing test plan
Status: RESOLVED FIXED
Product: gdk-pixbuf
Classification: Platform
Component: general
2.30.x
Other Linux
: Normal normal
: ---
Assigned To: gdk-pixbuf-maint
gdk-pixbuf-maint
Depends on:
Blocks:
 
 
Reported: 2015-09-01 09:37 UTC by Michael Biebl
Modified: 2016-04-11 00:21 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Skip tests when we can't run them due to lack of memory (4.46 KB, patch)
2015-09-20 12:49 UTC, Iain Lane
committed Details | Review
why subsample if scaling by such a small factor? (432 bytes, patch)
2015-12-17 15:10 UTC, Steven Chamberlain
none Details | Review

Description Michael Biebl 2015-09-01 09:37:19 UTC
Version: 2.31.6

Since the update to 2.31.6, I'm getting the following test-suite failure during "make check":

ERROR: cve-2015-4491 - missing test plan
ERROR: cve-2015-4491 - exited with status 133 (terminated by signal 5?)

Complete test-suite.log:
=============================================
   gdk-pixbuf 2.31.6: tests/test-suite.log
=============================================

# TOTAL: 77
# PASS:  75
# SKIP:  0
# XFAIL: 0
# FAIL:  0
# XPASS: 0
# ERROR: 2

.. contents:: :depth: 2

ERROR: cve-2015-4491
====================

# random seed: R02S6c485ba3929bcdd756dc05737afee2ab
# Start of pixbuf tests

(/home/michael/debian/build-area/gdk-pixbuf-2.31.6/tests/.libs/lt-cve-2015-4491:22262): GLib-ERROR **: /tmp/buildd/glib2.0-2.44.1/./glib/gmem.c:103: failed to allocate 6039798016 bytes
# Start of cve-2015-4491 tests
Trace/breakpoint trap
# GLib-FATAL-ERROR: /tmp/buildd/glib2.0-2.44.1/./glib/gmem.c:103: failed to allocate 6039798016 bytes
ERROR: cve-2015-4491 - missing test plan
ERROR: cve-2015-4491 - exited with status 133 (terminated by signal 5?)
Comment 1 Matthias Clasen 2015-09-01 12:25:01 UTC
try 2.31.7
Comment 2 Michael Biebl 2015-09-01 12:31:58 UTC
Same problem with 2.31.7:

# random seed: R02Sa65e62755ec50970d21846c60e6a0230
# Start of pixbuf tests

(/home/michael/debian/build-area/gdk-pixbuf-2.31.7/tests/.libs/lt-cve-2015-4491:16691): GLib-ERROR **: /tmp/buildd/glib2.0-2.44.1/./glib/gmem.c:103: failed to allocate 6039798016 bytes
# Start of cve-2015-4491 tests
Trace/breakpoint trap
# GLib-FATAL-ERROR: /tmp/buildd/glib2.0-2.44.1/./glib/gmem.c:103: failed to allocate 6039798016 bytes
ERROR: cve-2015-4491 - missing test plan
ERROR: cve-2015-4491 - exited with status 133 (terminated by signal 5?)


Re-opening bug reportg
Comment 3 Michael Biebl 2015-09-01 21:42:50 UTC
fwiw, this is blocking the update of gdk-pixbuf in Debian, since the failing test-suite prevents the package from building successfully.
Comment 4 Matthias Clasen 2015-09-02 01:24:58 UTC
I'm sure you know how to work around a failing test.
Comment 5 David King 2015-09-03 09:25:39 UTC
The test attempts to create a large pixmap, and fails in your case because it runs out of memory. It seems to need about 6 GB, so I would suggest disabling the test if there is not that much RAM available on the test machine (or upgrading the test machine).
Comment 6 Michael Biebl 2015-09-03 21:51:09 UTC
(In reply to David King from comment #5)
> The test attempts to create a large pixmap, and fails in your case because
> it runs out of memory. It seems to need about 6 GB, so I would suggest
> disabling the test if there is not that much RAM available on the test
> machine (or upgrading the test machine).

No, I don't think that's the case. I do have 8GB of RAM + a few GB of swap.
To me it looks like there is a leak somewhere so no matter how much memory I throw at it, the test will always (eventually) run out of memory and fail.

[  292.756619] Out of memory: Kill process 12340 (lt-cve-2015-449) score 966 or sacrifice child
[  292.756916] Killed process 12340 (lt-cve-2015-449) total-vm:11838928kB, anon-rss:7764984kB, file-rss:8kB

I also don't think disabling a test which was added for a CVE fix is the right approach here. Something is wrong here. And I'm not sure if the test is faulty or if it's a legitimate bug in gdk-pixbuf (or someplace else).
Comment 7 Iain Lane 2015-09-16 12:32:41 UTC
I feel bad because I uploaded this to Debian without noticing the bug - guess I have enough memory on my machine.

I made a memory limited container to reproduce the bug (running the testsuite like this `MALLOC_PERTURB_=$((${RANDOM:-256} % 256)) GDK_PIXBUF_MODULE_FILE=../gdk-pixbuf/loaders.cache ../libtool --mode=execute gdb ./cve-2015-4491') -- not sure why the MALLOC_PETURB_ is necessary; the test *passes* without it.

and got the following trace. Is there anything to be done about this?

  • #0 g_logv
    at /build/glib2.0-hcw3A1/glib2.0-2.45.7/./glib/gmessages.c line 324
  • #1 g_logv
    at /build/glib2.0-hcw3A1/glib2.0-2.45.7/./glib/gmessages.c line 1081
  • #2 g_log
    at /build/glib2.0-hcw3A1/glib2.0-2.45.7/./glib/gmessages.c line 1119
  • #3 g_malloc
    at /build/glib2.0-hcw3A1/glib2.0-2.45.7/./glib/gmem.c line 99
  • #4 g_malloc_n
    at /build/glib2.0-hcw3A1/glib2.0-2.45.7/./glib/gmem.c line 330
  • #5 bilinear_magnify_make_weights
    at pixops.c line 1550
  • #6 _pixops_scale
    at pixops.c line 2312
  • #7 _pixops_scale
    at pixops.c line 2381
  • #8 gdk_pixbuf_scale
    at gdk-pixbuf-scale.c line 156
  • #9 gdk_pixbuf_scale_simple
    at gdk-pixbuf-scale.c line 344
  • #10 get_scaled_pixbuf
    at gdk-pixbuf-scaled-anim.c line 138
  • #11 load_from_stream
    at gdk-pixbuf-io.c line 1501
  • #12 gdk_pixbuf_new_from_stream_at_scale
    at gdk-pixbuf-io.c line 1569
  • #13 gdk_pixbuf_new_from_resource_at_scale
    at gdk-pixbuf-io.c line 1805
  • #14 test_original
    at cve-2015-4491.c line 30
  • #15 g_test_run_suite_internal
    at /build/glib2.0-hcw3A1/glib2.0-2.45.7/./glib/gtestutils.c line 2158
  • #16 g_test_run_suite_internal
    at /build/glib2.0-hcw3A1/glib2.0-2.45.7/./glib/gtestutils.c line 2241
  • #17 g_test_run_suite_internal
    at /build/glib2.0-hcw3A1/glib2.0-2.45.7/./glib/gtestutils.c line 2253
  • #18 g_test_run_suite_internal
    at /build/glib2.0-hcw3A1/glib2.0-2.45.7/./glib/gtestutils.c line 2253
  • #19 g_test_run_suite
    at /build/glib2.0-hcw3A1/glib2.0-2.45.7/./glib/gtestutils.c line 2328
  • #20 g_test_run
    at /build/glib2.0-hcw3A1/glib2.0-2.45.7/./glib/gtestutils.c line 1596
  • #21 main
    at cve-2015-4491.c line 86

Comment 8 Iain Lane 2015-09-16 13:26:58 UTC
Maybe MALLOC_PERTURB initialising all of that huge chunk of memory forces it to be in RAM or something?

Setting malloc (M_PERTURB, 0) in this test works around the problem. Is that acceptable?
Comment 9 Iain Lane 2015-09-16 17:01:51 UTC
There's a similar case when even memory+swap isn't big enough (I think this is the reason anyway - it's on a Debian arm test machine).

  ERROR:cve-2015-4491.c:33:test_original: assertion failed (err == NULL): Not enough memory to load bitmap image (gdk-pixbuf-error-quark, 1)

Should we catch this and skip the test? If you can't allocate enough memory then the bug can't happen anyway, right?
Comment 10 Michael Biebl 2015-09-16 17:31:33 UTC
How much memory is required to run the test-suite?
I have 8GB of RAM + 4GB of swap?
Comment 11 Benjamin Otte (Company) 2015-09-19 19:52:11 UTC
commit 19f9685dbff7d1f929c61cf99188df917a18811d
Author: Benjamin Otte <otte@redhat.com>
Date:   Sat Sep 19 21:24:34 2015 +0200

    pixops: Fail make_weights functions on OOM
        
    The weights could grow very large under certain circumstances, in
    particular in security-relevant conditions, including the
    testsuite.
    By allowing the weight allocation to fail, this can be
    worked around.
Comment 12 Iain Lane 2015-09-20 12:49:30 UTC
Created attachment 311697 [details] [review]
Skip tests when we can't run them due to lack of memory

Check if we have failed due to insufficient memory and skip if so. Don't use MALLOC_PERTURB since it seems that this forces <size of file> memory to be available which OOMs on many systems.
Comment 13 Iain Lane 2015-09-20 12:50:21 UTC
Some of the tests still fail because they can't allocate in other places.

Can we have a patch like this to check for such failures and skip?
Comment 14 Michael Biebl 2015-09-20 12:52:55 UTC
How much memory is a system supposed to have to run the tests successfully?
I wouldn't consider 8G of RAM + 4G ofswap a weak meachine.
Comment 15 Benjamin Otte (Company) 2015-09-20 13:55:02 UTC
1. Commit 19f9685dbff7d1f929c61cf99188df917a18811d now uses g_try_malloc() and handles the case where it fails. So the tests should now always succeed. If they don't, could you please point out which ones fail and how?

2. The reason I want to keep the test as-is (even if that messes with swap on various build slaves) is that that test is the original test that was submitted as part of the CVE. And I'd like the testsuite to ensure we're not vulnerable to that.

3. The test allocates at most 6GB at once and a total of 16GB over its whole runtime (the rest being 2GB chunks). However, the test never allocated more than those 6GB at once and properly frees them before doing the other large allocations (I checked that with valgrind).
Comment 16 Michael Biebl 2015-09-20 14:28:23 UTC
fwiw, with latest git master, including Benjamin's commit, the test-suite now passes for me.

tests/cve-2015-4491.log now says:

# random seed: R02S4f81e707d6a26cf424966cb0b2eb3264
# Start of pixbuf tests
# Start of cve-2015-4491 tests
ok 1 /pixbuf/cve-2015-4491/original
PASS: cve-2015-4491 1 /pixbuf/cve-2015-4491/original
ok 2 /pixbuf/cve-2015-4491/scale-overflow
PASS: cve-2015-4491 2 /pixbuf/cve-2015-4491/scale-overflow
ok 3 /pixbuf/cve-2015-4491/scale-x-overflow
PASS: cve-2015-4491 3 /pixbuf/cve-2015-4491/scale-x-overflow
ok 4 /pixbuf/cve-2015-4491/scale-y-overflow
PASS: cve-2015-4491 4 /pixbuf/cve-2015-4491/scale-y-overflow
# End of cve-2015-4491 tests
# End of pixbuf tests
1..4
Comment 17 Iain Lane 2015-09-20 16:06:51 UTC
eder.debian.org (https://db.debian.org/machines.cgi?host=eder), which is similar to our mips build machines, hits these g_test_skips that I put in (master + my patch).

SKIP: cve-2015-4491 1 /pixbuf/cve-2015-4491/original # SKIP Your system has insufficient memory to load the test image.

SKIP: pixbuf-scale 9 /pixbuf/scale/png/large # SKIP Your system has insufficient memory to load the test image.
SKIP: pixbuf-scale 10 /pixbuf/scale/jpeg/large # SKIP format not supported (okay, not this one)
SKIP: pixbuf-scale 11 /pixbuf/add-alpha/large # SKIP Your system has insufficient memory to load the test image.
SKIP: pixbuf-scale 12 /pixbuf/rotate/large # SKIP Your system has insufficient memory to load the test image.

Does the "original" testcase exactly match the CVE bug? If so, then this machine won't be vulnerable and if not then maybe we can make the testcase require less memory (or add another one which takes less and let this one be skipped)?
Comment 18 Benjamin Otte (Company) 2015-09-20 17:38:23 UTC
Yes, the original test matches it exactly. I added a bunch of similar tests in the same source file that try to exploit the vulnerability while using less memory. Those tests seem to pass on your box.
Comment 19 Iain Lane 2015-09-20 19:18:40 UTC
Yeah, I've not got any code to skip those in that patch (they pass since the referenced commit but did also fail before - I think that if we looked we'd see that they are hitting the bail-out case now).

Can I commit this then?
Comment 20 Benjamin Otte (Company) 2015-09-22 14:03:42 UTC
Comment on attachment 311697 [details] [review]
Skip tests when we can't run them due to lack of memory

HA! I misread the test output for the cve test: The 6GB allocation happens when loading the image, not when scaling it.

In any case, I've committed most of the patch as commit e7f940102d40997f2e23a0589247cfb189dfaa98:

- I renamed and changed the behavior of the function checking for OOM, so that we can do proper cleanup and release memory.

- I did not commit the mallopt() call. For a start it's Linux-specific but more importantly from reading M_PERTURB docs I don't get what M_PERTURB has to do with all of this. And I'd like the testsuite to behave like the real app and setting malloc flags is exactly the wrong thing for that.
Plus, I don't think it's needed at all.

- Various indentation fixage.
Comment 21 Iain Lane 2015-09-22 15:05:53 UTC
Thanks! I'll try an upload to Debian with this patch.

I did add M_PERTURB for a reason, even if I didn't fully understand it (comment #8). Real apps wouldn't usually set this - for us it's set in glib-tap,mk for the testsuite only. So I considered it okay to turn this off. Anyway, hopefully the other fix has made this part obsolete but we'll see.

I'll close this bug if it builds for all of our architecture.
Comment 22 Iain Lane 2015-09-23 08:46:59 UTC
https://buildd.debian.org/status/package.php?p=gdk-pixbuf&version=2.32.0-1

Seems fine for the architectures we care about.

I think the failure for "kfreebsd-amd64" there is an OOM. If it starts to happen on more architectures then we need to look at this more, but it doesn't matter so much for now.
Comment 23 Steven Chamberlain 2015-12-17 15:09:09 UTC
Hi,

Isn't it kind of sub-optimal to do a 16x sub-sampled bilinear scale, even when the scale factor is smaller than 1/16 anyway?  Doing so here requires an extra 6 GiB memory allocation for make_weights.

I'm saying, why not set a threshold in _pixops_scale_real to just use PIXOPS_INTERP_NEAREST if either of the scale factors is really tiny?  It is more robust than failing due to memory exhaustion, ought to be a lot faster, and likely would have prevented CVE-2015-4491.

Thanks.
Comment 24 Steven Chamberlain 2015-12-17 15:10:35 UTC
Created attachment 317575 [details] [review]
why subsample if scaling by such a small factor?
Comment 25 Matthias Clasen 2016-04-11 00:21:37 UTC
attaching patches 5o a resolved bug is not effective; please open a new bug for this