mount --bind accounting

mount --bind accounting

[Andries.Brouwer]

Something entirely different.

Last year I added the comment
	/* No capabilities? What if users do thousands of these? */
in super.c for "mount --bind".
Now that I do some polishing there I noticed the comment again.

Each bind does an alloc_vfsmnt() and hence takes some kernel memory.
Any user can therefore take all kernel memory, until
	kmalloc(sizeof(struct vfsmount), GFP_KERNEL)
fails. Bad security.
I suppose something needs to be done about that.

Invent an arbitrary limit
	#define MAX_NUMBER_OF_USER_BINDS 666
and refuse the "mount --bind" when inspection of the mnt_owner fields
of the entries in the vfsmntlist shows that this user already has
too many mounts?

Andries

Re: mount --bind accounting

[Alexander Viro]

On Sun, 3 Jun 2001 Andries.Brouwer@cwi.nl wrote:

> Something entirely different.
> 
> Last year I added the comment
> 	/* No capabilities? What if users do thousands of these? */
> in super.c for "mount --bind".
> Now that I do some polishing there I noticed the comment again.
> 
> Each bind does an alloc_vfsmnt() and hence takes some kernel memory.
> Any user can therefore take all kernel memory, until
> 	kmalloc(sizeof(struct vfsmount), GFP_KERNEL)
> fails. Bad security.
> I suppose something needs to be done about that.

Like looking at mount_is_safe(), maybe?

Re: mount --bind accounting

[Stephen C. Tweedie]

Hi,

On Sun, Jun 03, 2001 at 10:38:29PM +0200, Andries.Brouwer@cwi.nl wrote:

> Each bind does an alloc_vfsmnt() and hence takes some kernel memory.
> Any user can therefore take all kernel memory, until
> 	kmalloc(sizeof(struct vfsmount), GFP_KERNEL)
> fails. Bad security.

Until we can account properly for basic things like page tables, the
small kmallocs for things like vfsmount and file structs will be
negligible in comparison.

Fortunately we used to have at least skeleton patches for a framework
in which to do this.  Whatever happened to beancounter, anyway?  Is
somebody maintaining that at all these days?

Cheers,
 Stephen

Re: mount --bind accounting

[Andries.Brouwer]

>> /* No capabilities? What if users do thousands of these? */

> look at mount_is_safe()

Yes, good. My remark means that more tests are required
than those sketched in mount_is_safe(), and that means
that for the time being we can throw out the routine
mount_is_safe(), and remove the test on capable(CAP_SYS_ADMIN)
in do_remount(), and move the same test in do_mount up to
the start: all forms of mount require CAP_SYS_ADMIN.

[side effect: remount read-only upon umount of root fs
may be possible in a few more cases]

Andries

Re: mount --bind accounting

[Alexander Viro]

On Mon, 4 Jun 2001 Andries.Brouwer@cwi.nl wrote:

> >> /* No capabilities? What if users do thousands of these? */
> 
> > look at mount_is_safe()
> 
> Yes, good. My remark means that more tests are required
> than those sketched in mount_is_safe(), and that means
> that for the time being we can throw out the routine
> mount_is_safe(), and remove the test on capable(CAP_SYS_ADMIN)
> in do_remount(), and move the same test in do_mount up to
> the start: all forms of mount require CAP_SYS_ADMIN.
> 
> [side effect: remount read-only upon umount of root fs
> may be possible in a few more cases]

IMO umount / is bogus. For 2.5 we have much better way to unmount
everything - as soon as rootfs patch goes in, we will be able to
unmount root for real and just fall back to absolute root. Besides,
we can simply pass MNT_DETACH to umount(2) and wait until everything
goes quiet (see namespace-patch). That has a nice side effect - if
some fs is really busy (hung NFS hard mount, leak somewhere, whatever)
it will not hold the filesystem it's mounted on. Or prevent clean
unmount of filesystems mounted under it, for that matter, even if
we would hang trying to look their mountpoints up. Whether it goes
into the distributions' shutdown sequences or not, I'm quite happy
using it when I do fs stuff - I'm sick and tired of forced fsck on
root just because experimental stuff mounted on /mnt decides to
hang...