[PATCH v2] mount: mount--move with mtab support fix

Rob Landley rob at landley.net
Wed Feb 23 05:09:37 UTC 2011


On 02/22/2011 08:10 AM, Roman Borisov wrote:
>>> v2: added #if ENABLE_FEATURE_MTAB_SUPPORT in functions declarations;
>>> added ENABLE_FEATURE_CLEAN_UP condition check for cleaning
>>
>> If this is only ever used in one place, why does it live in libbb
>> instead of in mount.c?
>>
>> The erase_mtab() function was shared by mount and umount.  This isn't.
>>
> 
> Thanks for historic tour; so update_mtab() must be moved to mount.c like
> erase_mtab() must be moved to umount.c; Another way - to combine this
> functionality to one function.

Dunno about "must", but keeping the code small and clean is sort of the
point of busybox.  (Otherwise you'd just use the gnu stuff.)

>> And then the compiler's dead code elimination deletes the reference
>> (this is why we have function-secitons and gc-sections in the compiler),
>> but the code is always syntax checked the same way and changing the
>> config doesn't introduce build breaks.
>>
> 
> Thanks, I tried to change something after the comment on initial patch.
> Please review "[PATCH] mount: mount--move with mtab support fix". It
> doesn't contain "#if ENABLE_FEATURE_MTAB_SUPPORT".

It's a separate hunk, the rest of this patch doesn't depend on that.
(You can prototype functions you never use or include, otherwise the
standard headers wouldn't work.)

>>> +input mntent and replace it by new one.
>>> +*/
>>> +void FAST_FUNC update_mtab_entry(const struct mntent *mp)
>>> +{
[big long function removed]
>>>   #endif
>>
>> There is still a situation where symlinking /etc/mtab to /proc/mounts
>> isn't the right thing to do?  All these years after the introduction of
>> shared subtrees?
>>
>> Isn't the /etc/mtab support broken legacy crap, to which you're patching
>> a single weird corner case out of the 30 different ways it's broken at
>> the _design_ level?
>>
> 
> We suppose that all code distributed must be working.

Our tune2fs doesn't have ext3 support, just ext2.  That doesn't mean
it's not working, just "it doesn't have every feature the gnu version does".

There are dozens of things you can do that break /etc/mtab.  That's why
the kernel started exporting this information via /proc/mounts.  The
mount tree has been a per-process attribute ever since CLONE_NEWNS went
in circa 2.4.19 (see the clone(2) man page).  Then the unshare(2) system
call got added:

  http://lwn.net/Articles/155462/

Then the shared subtree stuff extended that circa 2.6.15:

  http://lwn.net/Articles/159077/

Of course the security guys went nuts with all this stuff:


http://www.chromium.org/chromium-os/chromiumos-design-docs/system-hardening
  http://www.debian-administration.org/article/628/Per-Process_Namespaces
  http://www.ibm.com/developerworks/linux/library/l-mount-namespaces.html

But you don't need that to screw up mtab.  A simple chroot will do it
quite handily.

Basically, /etc/mtab /etc/mtab support is a relic that doesn't work with
a large range of modern Linux features.  It was retained basically
because some people like to hack up their kernel to remove procfs, so
there's no /proc/mounts to symlink /etc/mtab to.  (Which also means you
can't do things like get a list of running processes unless you add
additional nonstandard hacks to your kernel, but the people who made
Vmware ESX were crazy enough to do just that, so...)

The legacy mtab support hasn't seen a whole lot of love because it's
legacy crap that is fundamentally broken in a variety of ways.  You
propose to add a large function to fix _one_ of those ways.

> Do you think to
> exclude --move and --bind functionality in case of MTAB_SUPPORT?

Yes.

> Ok.
> Let's return proper error code or/and document it.

Or, you could read help text on FEATURE_MTAB_SUPPORT that's been there
for about five years now:

config FEATURE_MTAB_SUPPORT
        bool "Support for the old /etc/mtab file"
        default n
        depends on MOUNT || UMOUNT
        select FEATURE_MOUNT_FAKE
        help
          Historically, Unix systems kept track of the currently mounted
          partitions in the file "/etc/mtab". These days, the kernel exports
          the list of currently mounted partitions in "/proc/mounts",
rendering
          the old mtab file obsolete. (In modern systems, /etc/mtab
should be
          a symlink to /proc/mounts.)

          The only reason to have mount maintain an /etc/mtab file
itself is if
          your stripped-down embedded system does not have a /proc
directory.
          If you must use this, keep in mind it's inherently brittle (for
          example a mount under chroot won't update it), can't handle modern
          features like separate per-process filesystem namespaces, requires
          that your /etc directory be writable, tends to get easily confused
          by --bind or --move mounts, won't update if you rename a directory
          that contains a mount point, and so on. (In brief: avoid.)

          About the only reason to use this is if you've removed /proc from
          your kernel.

Documenting all the ways /etc/mtab _not_ being a symlink to /proc/mounts
is broken on 2.6 kernels is a losing proposition because they keep
inventing NEW ways with each kernel release, but that lists a couple
that I'd forgotten.

Rob


More information about the busybox mailing list