CodethinkLabs / sandboxlib

Sandboxing library
8 stars 4 forks source link

do we really need the bind functionality? #9

Open devcurmudgeon opened 9 years ago

devcurmudgeon commented 9 years ago

i've noticed that if the directory structure changes (eg another program deletes a directory while sandboxlib is running), we can hit 'mount (MS_BIND): No such file or directory'

ssssam commented 9 years ago

There are two types of bind mount done by the linux-user-chroot backend.

1 is using linux-user-chroot's --mount-bind for things from the host that you want to make available in the sandbox. In the case of YBD, this is used for the 'ccache' directory. The sandboxlib.linux_user_chroot module also uses --mount-bind to set up a 'tmpfs' mount inside the sandbox.

The 2nd type is --mount-readonly. This is used for making certain paths inside the sandbox readonly. Morph and YBD (with sandboxlib.linux_user_chroot backend) both make the whole sandbox readonly apart from the /chunk.build and /chunk.inst directories. This is useful as a way of spotting if a chunk's install-commands ignore the DESTDIR environment variable, and when the staging area is created from hardlinks (both Morph and YBD do this) it's vital to avoid anything corrupting the unpacked chunk artifacts in the cache. E.G. if I run make install and it overwrites /bin/cp instead of /chunk.inst/bin/cp, then the file /src/cache/artifacts/busybox.123456.unpacked/bin/cp will be altered, and subsequent builds that use that chunk will behave in randomly different ways than expected. I've lost days in the past to having a corrupted unpacked chunk artifact. In 'bootstrap' build mode the build can see the whole system, so the problem is even worse: it can overwrite stuff from the actual host system.

I don't know the exact details of the problem you're seeing, but it may be due to the really stupid amount of --mount-readonly flags we have to pass to linux-user-chroot. The problem is that there's no way to say to linux-user-chroot 'make everything readonly except these files and directories'. Instead, we have to say 'make all these files and directories read-only', and the resulting list is huge. It's quite possible that if a directory gets deleted inbetween sandboxlib constructing the list and linux-user-chroot processing it, you'll see an error, that may be the problem here.

To fix this, I can only think of one option: add a --read-only option to linux-user-chroot that would make everything readonly except the things we explicitly bind in with --mount-bind. Downside is that it might take forever for the new version of linux-user-chroot to end up in distros, unless we did the work of releasing and packaging it everywhere ourselves.

richardmaw-codethink commented 9 years ago

On Thu, Jul 02, 2015 at 01:49:03AM -0700, Sam Thursfield wrote:

I don't know the exact details of the problem you're seeing, but it may be due to the really stupid amount of --mount-readonly flags we have to pass to linux-user-chroot. The problem is that there's no way to say to linux-user-chroot 'make everything readonly except these files and directories'. Instead, we have to say 'make all these files and directories read-only', and the resulting list is huge. It's quite possible that if a directory gets deleted inbetween sandboxlib constructing the list and linux-user-chroot processing it, you'll see an error, that may be the problem here.

For reference, the huge list of --mount-readonly binds was the last resort.

It should be possible to make the root bind read-only and explicitly mark sub-paths as writable, however while the Kernel's ABI policy is officially only broken in exceptional cases, like security vulnerabilities; unofficially it's a case of if nobody is testing a specific feature and feeding back the changes then things change without notice.

Bind-mount behaviour was in this grey area when I did this investigation. On some systems I could mount the staging area's root read-only then bind-mount the subpaths I wanted writable and have it work. On others this didn't, so mounting everything that you don't want writable was the only thing that worked consistently.

To fix this without making YBD more susceptible to corruption of the .unpacked artifacts, I can only think of one option really: add a --read-only option to linux-user-chroot that would make everything readonly except the things we explicitly bind in with --mount-bind. Downside is that it might take forever for the new version of linux-user-chroot to end up in distros, unless we did the work of releasing and packaging it ourselves everywhere.

If the problem is that there's too many binds for the old version's arbitrary limit, then we wouldn't need to make any changes to get it to work, just time for it to percolate into distros.