GNOME Bugzilla – Bug 775896
rgb888amsb() is broken on Big Endian, and crashes on Little Endian
Last modified: 2017-09-22 09:20:40 UTC
Created attachment 341686 [details] [review] Patch to fix rgb888amsb() On Little Endian, it writes memory out of bounds (i.e. when running inside a little endian client, connecting to a big endian X server). On Big Endian, it only processes the first 1/4 of an icon. The effects can be seen when running IceWM. Running native on Big Endian ppc64 with a Big Endian X11 server, the icon background was 3/4 random: https://bugzilla.suse.com/show_bug.cgi?id=929462 This match makes rgb888amsb() behave analogously to rgb888msb().
Can you please attach a test case for the problem as well? Something that doesn't involve running icewm would be nice.
Created attachment 341890 [details] Minimal test case to use rgb888amsb() Here is a test case, with instructions and explanations inside. Note that you need to connect to a big-endian X server to reproduce this - such as an old-school PowerPC, or ppc64, but *not* on ppc64le. I'm honestly surprised this test case is necessary, as the code in rgb888amsb() is clearly unfinished and will perform out-of-bounds writes in the #ifdef LITTLE case. I hope it helps in clarifying the problem though.
From the test case, there's at least one problem in the code. It's completely expected that uninitialised data is written, as the pixbuf was never initialised. You probably want to run gdk_pixbuf_fill() before showing it. I've asked someone with access to PPC to integrate this into the tests and verify the patch.
(In reply to Max Staudt from comment #2) <snip> > I'm honestly surprised this test case is necessary, as the code in > rgb888amsb() is clearly unfinished and will perform out-of-bounds writes in > the #ifdef LITTLE case. I hope it helps in clarifying the problem though. I couldn't see anything wrong, with your test case, on little endian (the out-of-bounds writes you mentioned). I'll be working to integrate your test case into the gdk-pixbuf test suite.
(In reply to Bastien Nocera from comment #4) > I couldn't see anything wrong, with your test case, on little endian (the > out-of-bounds writes you mentioned). I'll be working to integrate your test > case into the gdk-pixbuf test suite. To reproduce this, you need to run the X client on a little endian machine, and connect it to an X server which runs on a big endian machine.
In fact, this function rgb888amsb() is only used (as far as I can tell) when the X server is running on a big endian machine. The way in which it misbehaves depends on whether the client (of which gdk-pixbuf is a part of) is running on a little endian machine, or on a big endian machine. On a little endian machine, you'll get out-of-bounds accesses, on big endian you'll get a pixmap that's 1/4 white and 3/4 uninitialized data. What's really important is that the X server runs on a big endian machine, otherwise this code is never called. Thanks a bunch for looking into this!
I never managed to get someone to test this out, so I'll take your word for it that your patch fixes the problem. We might be removing this code soon, and dependencies will have to start using their own code for this instead. This code was never fully supported as part of gdk-pixbuf (as the path shows), and it's been on life support ever since.
Thanks for taking the fix!