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 787072 - gnome-desktop thumbnailing assumes merged /usr
gnome-desktop thumbnailing assumes merged /usr
Status: RESOLVED FIXED
Product: gnome-desktop
Classification: Core
Component: Thumbnail
3.25.x
Other Linux
: Normal normal
: ---
Assigned To: Desktop Maintainers
Desktop Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-08-31 10:36 UTC by Simon McVittie
Modified: 2017-12-28 23:05 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
thumbnailer: also add /lib(64)? to bwrap's link-mounted directory list (1.25 KB, patch)
2017-09-01 22:07 UTC, Dominique Leuenberger
committed Details | Review

Description Simon McVittie 2017-08-31 10:36:47 UTC
+++ This bug was initially created as a clone of Bug #785963 +++


> About to launch script: bwrap --ro-bind /usr /usr --proc /proc --dev /dev
> --symlink usr/lib /lib --symlink usr/lib64 /lib64 --symlink usr/bin /bin
> --symlink usr/sbin /sbin ...

This is not going to work on a system where the /usr merge (<https://wiki.debian.org/UsrMerge>, <https://www.freedesktop.org/wiki/Software/systemd/TheCaseForTheUsrMerge/>) has not yet been carried out. Binaries tend to work poorly when they can't see /lib64/ld-linux-x86-64.so.2 or equivalent :-)

Debian >= 9 has support for the /usr merge (unless one of a couple of still-broken packages are installed), but does not carry it out by default, either on existing installations or on upgraded systems.

One way to handle this would be something like this pseudocode:

    for directory in lib lib64 bin sbin
    do
        if ! test -e /$directory
        then
            continue
        fi

        if test /$directory -ef /usr/$directory
        then
            append --symlink usr/$directory /$directory to bwrap command
        else
            append --ro-bind /$directory /$directory to bwrap command
        fi
    done

where test -ef consists of stat()ing the two operands (stat() not lstat()!) and checking whether their device and inode numbers match.

Another way would be to skip the use of bwrap if we detect that the /usr merge has not been carried out.

> Of course, Ubuntu and Debian don't use lib64.

They do not use lib64 for ordinary libraries, but on 64-bit platforms they still need to see the dynamic linker at /lib64/ld-linux-x86-64.so.2 or similar, because the cross-distribution ABIs for 64-bit architectures say that's the path that is required to be hard-coded into binaries.
Comment 1 Dominique Leuenberger 2017-09-01 22:07:17 UTC
Created attachment 358960 [details] [review]
thumbnailer: also add /lib(64)? to bwrap's link-mounted directory list

As not all distros have libs moves from /lib(64)? to /usr/lib(64), we need
to also bind-mount those directories, or the thumbnailers will fail to load
the required libraries.
Comment 2 Dominique Leuenberger 2017-09-01 22:08:15 UTC
(In reply to Dominique Leuenberger from comment #1)
> Created attachment 358960 [details] [review] [review]
> thumbnailer: also add /lib(64)? to bwrap's link-mounted directory list

NOTE: I test drove that on an openSUSE Tumbleweed system where things 'seem' to work out fine now for us - but that does not mean a lot: before it worked fine for other users :)
Comment 3 Dominique Leuenberger 2017-09-01 22:12:56 UTC
generally, I'm not sure why users that have /usr merge done would need the symlink /usr/lib /lib - that sounds quite strange to me
Comment 4 Simon McVittie 2017-09-01 23:17:05 UTC
(In reply to Dominique Leuenberger from comment #3)
> generally, I'm not sure why users that have /usr merge done would need the
> symlink /usr/lib /lib

Because many executables, and many specifications, hard-code a path starting with either /lib or /lib64. On a system with merged /usr, /lib is equivalent to /usr/lib and /lib64 is equivalent to /usr/lib64, and that's fine; but having those paths just not exist at all is not an option.

In particular, the x86 Linux ABI says all 32-bit x86 binaries must have the dynamic linker /lib/ld-linux.so.2 hard-coded into the executable in the ELF interpreter field, so you cannot execute a 32-bit ELF binary without /lib (it might be a symlink to /usr/lib, meaning you actually get /usr/lib/ld-linux.so.2, but it must exist); and similarly, 64-bit x86 binaries have ELF interpreter /lib64/ld-linux-x86-64.so.2, so 64-bit systems need /lib64 (even on systems like Debian that don't normally use a directory named lib64 for *libraries*, they still need it for the dynamic linker). Similar considerations apply to other CPU architectures, although the precise names differ.

(In reply to Dominique Leuenberger from comment #2)
> NOTE: I test drove that on an openSUSE Tumbleweed system

Is that a system where the /usr merge has already been carried out, meaning /lib is a symbolic link to /usr/lib and /lib64 is a symbolic link to /usr/lib64?

> 	    "--ro-bind", "/lib64", "/lib64",

I suspect this is going to cause bwrap to fail on a pure 32-bit platform that does not have /lib64 at all (such as i386 or ARM without multilib), although I might be wrong about that.
Comment 5 Bastien Nocera 2017-09-02 16:33:22 UTC
(In reply to Dominique Leuenberger from comment #2)
> (In reply to Dominique Leuenberger from comment #1)
> > Created attachment 358960 [details] [review] [review] [review]
> > thumbnailer: also add /lib(64)? to bwrap's link-mounted directory list
> 
> NOTE: I test drove that on an openSUSE Tumbleweed system where things 'seem'
> to work out fine now for us - but that does not mean a lot: before it worked
> fine for other users :)

Line 788, you can enable some debug to see what command-line is being generated. Verify that the symlinks are just how you set them up.

test-desktop-thumbnail can be used to compare with an unwrapper thumbnailer (such as gdk-pixpuf-thumbnailer).

It would be good to have the original error in the commit message as well.

(In reply to Simon McVittie from comment #4)
<snip>
> > 	    "--ro-bind", "/lib64", "/lib64",
> 
> I suspect this is going to cause bwrap to fail on a pure 32-bit platform
> that does not have /lib64 at all (such as i386 or ARM without multilib),
> although I might be wrong about that.

Non-existent source directories will simply be ignored.
Comment 6 Bastien Nocera 2017-09-12 08:46:09 UTC
Dominique, I'm still waiting for a test on your part before pushing this...
Comment 7 Dominique Leuenberger 2017-09-12 08:51:16 UTC
Oh - I missed this; sorry.

well, as said: the patch as posted works on openSUSE systems, as I'd expect it.
I don't have any other system around to verify/validate if it does not break there (the patch is possibly too simplistic, as it bind-mounts /lib64 also on /usr-merged systems - this could or could not work)
Comment 8 Bastien Nocera 2017-09-12 09:10:07 UTC
Attachment 358960 [details] pushed as 886994f - thumbnailer: also add /lib(64)? to bwrap's link-mounted directory list
Comment 9 Jürg Billeter 2017-12-25 13:03:15 UTC
(In reply to Bastien Nocera from comment #5)
> Non-existent source directories will simply be ignored.

This doesn't appear to be the case. I see the following with bubblewrap 0.2.0:

$ bwrap --bind /foo /foo bar
bwrap: Can't find source path /foo: No such file or directory

This change broke systems without /lib64. I've noticed it on a x86-64 system without /lib64, which is uncommon, however, I expect it to fail the same way on other architectures such as x86-32 and ARMv7.
Comment 10 Bastien Nocera 2017-12-28 23:05:00 UTC
(In reply to Jürg Billeter from comment #9)
> (In reply to Bastien Nocera from comment #5)
> > Non-existent source directories will simply be ignored.
> 
> This doesn't appear to be the case.

It was the case in 0.1.8 so it's a new bug. Please file a separate bug and reference it here.