public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* mount --bind accounting
@ 2001-06-03 20:38 Andries.Brouwer
  2001-06-04  0:18 ` Alexander Viro
  2001-06-06 18:20 ` Stephen C. Tweedie
  0 siblings, 2 replies; 5+ messages in thread
From: Andries.Brouwer @ 2001-06-03 20:38 UTC (permalink / raw)
  To: alan, linux-kernel, viro

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: mount --bind accounting
  2001-06-03 20:38 mount --bind accounting Andries.Brouwer
@ 2001-06-04  0:18 ` Alexander Viro
  2001-06-06 18:20 ` Stephen C. Tweedie
  1 sibling, 0 replies; 5+ messages in thread
From: Alexander Viro @ 2001-06-04  0:18 UTC (permalink / raw)
  To: Andries.Brouwer; +Cc: alan, linux-kernel



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?


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: mount --bind accounting
@ 2001-06-04  0:59 Andries.Brouwer
  2001-06-04  1:11 ` Alexander Viro
  0 siblings, 1 reply; 5+ messages in thread
From: Andries.Brouwer @ 2001-06-04  0:59 UTC (permalink / raw)
  To: Andries.Brouwer, viro; +Cc: alan, linux-kernel

>> /* 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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: mount --bind accounting
  2001-06-04  0:59 Andries.Brouwer
@ 2001-06-04  1:11 ` Alexander Viro
  0 siblings, 0 replies; 5+ messages in thread
From: Alexander Viro @ 2001-06-04  1:11 UTC (permalink / raw)
  To: Andries.Brouwer; +Cc: alan, linux-kernel



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...


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: mount --bind accounting
  2001-06-03 20:38 mount --bind accounting Andries.Brouwer
  2001-06-04  0:18 ` Alexander Viro
@ 2001-06-06 18:20 ` Stephen C. Tweedie
  1 sibling, 0 replies; 5+ messages in thread
From: Stephen C. Tweedie @ 2001-06-06 18:20 UTC (permalink / raw)
  To: Andries.Brouwer; +Cc: alan, linux-kernel, viro, Stephen 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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2001-06-06 18:21 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2001-06-03 20:38 mount --bind accounting Andries.Brouwer
2001-06-04  0:18 ` Alexander Viro
2001-06-06 18:20 ` Stephen C. Tweedie
  -- strict thread matches above, loose matches on Subject: below --
2001-06-04  0:59 Andries.Brouwer
2001-06-04  1:11 ` Alexander Viro

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox