From: Miklos Szeredi <[email protected]>
Add a new filesystem flag, that results in the VFS not checking if the
current process has enough privileges to do an mknod().
This is needed on filesystems, where an unprivileged user may be able
to create a device node, without causing security problems.
One such example is "mountlo" a loopback mount utility implemented
with fuse and UML, which runs as an unprivileged userspace process.
In this case the user does in fact have the right to create device
nodes within the filesystem image, as long as the user has write
access to the image. Since the filesystem is mounted with "nodev",
adding device nodes is not a security concern.
This feature is basically "fuse-only", so it does not make sense to
change the semantics of ->mknod().
Signed-off-by: Miklos Szeredi <[email protected]>
---
Index: linux/fs/namei.c
===================================================================
--- linux.orig/fs/namei.c 2007-09-21 13:45:14.000000000 +0200
+++ linux/fs/namei.c 2007-09-21 13:45:16.000000000 +0200
@@ -1922,7 +1922,8 @@ int vfs_mknod(struct inode *dir, struct
if (error)
return error;
- if ((S_ISCHR(mode) || S_ISBLK(mode)) && !capable(CAP_MKNOD))
+ if (!(dir->i_sb->s_type->fs_flags & FS_MKNOD_CHECKS_PERM) &&
+ (S_ISCHR(mode) || S_ISBLK(mode)) && !capable(CAP_MKNOD))
return -EPERM;
if (!dir->i_op || !dir->i_op->mknod)
Index: linux/include/linux/fs.h
===================================================================
--- linux.orig/include/linux/fs.h 2007-09-21 13:45:14.000000000 +0200
+++ linux/include/linux/fs.h 2007-09-21 13:45:16.000000000 +0200
@@ -97,6 +97,7 @@ extern int dir_notify_enable;
#define FS_BINARY_MOUNTDATA 2
#define FS_HAS_SUBTYPE 4
#define FS_SAFE 8 /* Safe to mount by unprivileged users */
+#define FS_MKNOD_CHECKS_PERM 16 /* FS checks if device creation is allowed */
#define FS_REVAL_DOT 16384 /* Check the paths ".", ".." for staleness */
#define FS_RENAME_DOES_D_MOVE 32768 /* FS will handle d_move()
* during rename() internally.
--
On Fri, Sep 21, 2007 at 02:23:48PM +0200, Miklos Szeredi wrote:
> From: Miklos Szeredi <[email protected]>
>
> Add a new filesystem flag, that results in the VFS not checking if the
> current process has enough privileges to do an mknod().
>
> This is needed on filesystems, where an unprivileged user may be able
> to create a device node, without causing security problems.
A user should never be able to create devices. And no, I don't want to
see a filesystem that implements it's own file operations for device nodes.
> > From: Miklos Szeredi <[email protected]>
> >
> > Add a new filesystem flag, that results in the VFS not checking if the
> > current process has enough privileges to do an mknod().
> >
> > This is needed on filesystems, where an unprivileged user may be able
> > to create a device node, without causing security problems.
>
> A user should never be able to create devices.
A user can already create a device with fuse implicitly. This patch
would just allow that explicitly.
Take this example: I've loopback mounted an UML disk image using fuse
(no privileges required), and want to create some device nodes. I
can't yet boot the UML because the device node is missing from the
image. So what should I do. Currently I have to manipulate the
mounted image as root. But that's really shouldn't be needed.
> And no, I don't want to see a filesystem that implements it's own
> file operations for device nodes.
I don't want that either, and it has nothing to do with this patch.
Miklos
On Fri, Sep 21, 2007 at 03:10:26PM +0200, Miklos Szeredi wrote:
> Take this example: I've loopback mounted an UML disk image using fuse
> (no privileges required), and want to create some device nodes. I
> can't yet boot the UML because the device node is missing from the
> image. So what should I do. Currently I have to manipulate the
> mounted image as root. But that's really shouldn't be needed.
That's something that shouldn't be solved in the filesystem, but rather
through exact semantics of unprivilegued mounts. Given that an
unprivilegued implies ignoring the device files we can easily allow
users to create them, because they're nothing special anymore.
> > Take this example: I've loopback mounted an UML disk image using fuse
> > (no privileges required), and want to create some device nodes. I
> > can't yet boot the UML because the device node is missing from the
> > image. So what should I do. Currently I have to manipulate the
> > mounted image as root. But that's really shouldn't be needed.
>
> That's something that shouldn't be solved in the filesystem, but rather
> through exact semantics of unprivilegued mounts. Given that an
> unprivilegued implies ignoring the device files we can easily allow
> users to create them, because they're nothing special anymore.
Exacly. And we already have an API for that: mknod(2). It would be
quite stupid to introduce _another_ API to do the same. It would mean
that all the tools, like mknod(8) would not work with the new API.
Or am I misunderstanding your suggestion?
Miklos
On Fri, Sep 21, 2007 at 03:18:33PM +0200, Miklos Szeredi wrote:
> > That's something that shouldn't be solved in the filesystem, but rather
> > through exact semantics of unprivilegued mounts. Given that an
> > unprivilegued implies ignoring the device files we can easily allow
> > users to create them, because they're nothing special anymore.
>
> Exacly. And we already have an API for that: mknod(2). It would be
> quite stupid to introduce _another_ API to do the same. It would mean
> that all the tools, like mknod(8) would not work with the new API.
>
> Or am I misunderstanding your suggestion?
Yes :)
My suggestions is:
- mknod for unprivilegued user is allowed in the following case
(1) mount point is mounted with MNT_NODEV
(2) mount point is owner by the user doing mknod
- and maybe
(3) we have a special mount option to allow it if we don't want
to allow it for normal unprivilegued mounts for some reason
which implies we need to get in unprivilegued mounts first, but we'll
have to do that anyway.
> On Fri, Sep 21, 2007 at 03:18:33PM +0200, Miklos Szeredi wrote:
> > > That's something that shouldn't be solved in the filesystem, but rather
> > > through exact semantics of unprivilegued mounts. Given that an
> > > unprivilegued implies ignoring the device files we can easily allow
> > > users to create them, because they're nothing special anymore.
> >
> > Exacly. And we already have an API for that: mknod(2). It would be
> > quite stupid to introduce _another_ API to do the same. It would mean
> > that all the tools, like mknod(8) would not work with the new API.
> >
> > Or am I misunderstanding your suggestion?
>
> Yes :)
>
> My suggestions is:
>
> - mknod for unprivilegued user is allowed in the following case
>
> (1) mount point is mounted with MNT_NODEV
> (2) mount point is owner by the user doing mknod
Ah, OK. Well, that's what fuse would do with the above change. So
you are basically saying, the change is OK, but we want proper
unprivileged mounts first.
> - and maybe
>
> (3) we have a special mount option to allow it if we don't want
> to allow it for normal unprivilegued mounts for some reason
I'm sure we don't want it by default.
For example if user bind mounts / onto /home/user/myroot (with 'nodev'
of couse), we still don't want mknod to work on that mount, for
obvious reasons.
Miklos
On Fri, Sep 21, 2007 at 04:48:58PM +0200, Miklos Szeredi wrote:
> Ah, OK. Well, that's what fuse would do with the above change. So
> you are basically saying, the change is OK, but we want proper
> unprivileged mounts first.
Yes, that and that it should be a mount flag, not a file_system_type
flag.
> I'm sure we don't want it by default.
>
> For example if user bind mounts / onto /home/user/myroot (with 'nodev'
> of couse), we still don't want mknod to work on that mount, for
> obvious reasons.
True, we'll have to deny it if there is any non-privilegued mount of
the backing device possible. At this point it's getting rather nasty
and I wonder whether it's really worth it..
> On Fri, Sep 21, 2007 at 04:48:58PM +0200, Miklos Szeredi wrote:
> > Ah, OK. Well, that's what fuse would do with the above change. So
> > you are basically saying, the change is OK, but we want proper
> > unprivileged mounts first.
>
> Yes, that and that it should be a mount flag, not a file_system_type
> flag.
>
> > I'm sure we don't want it by default.
> >
> > For example if user bind mounts / onto /home/user/myroot (with 'nodev'
> > of couse), we still don't want mknod to work on that mount, for
> > obvious reasons.
>
> True, we'll have to deny it if there is any non-privilegued mount of
> the backing device possible. At this point it's getting rather nasty
> and I wonder whether it's really worth it..
I think the assumption, that we want this as a generic service is
false.
We want this as a special service for a few filesystems, such as the
unprivileged userspace loopback mounting I was talking about.
So my thinking is: if an unprivileged filesystem explicitly asks for
this, then it should be allowed. It could be a per-superblock flag
instead of a per fs-type flag, if that sounds better.
My fuse implementation would have been exactly the same: the ->mknod()
implementation would check a per filesystem flag, and if it's not set,
check the permissions as normal mknod() would. Here's the relevant
patch snippet:
Index: linux/fs/fuse/dir.c
===================================================================
--- linux.orig/fs/fuse/dir.c 2007-09-21 13:45:23.000000000 +0200
+++ linux/fs/fuse/dir.c 2007-09-21 13:45:25.000000000 +0200
@@ -486,7 +486,13 @@ static int fuse_mknod(struct inode *dir,
{
struct fuse_mknod_in inarg;
struct fuse_conn *fc = get_fuse_conn(dir);
- struct fuse_req *req = fuse_get_req(fc);
+ struct fuse_req *req;
+
+ if (!fc->mknod_nocheck &&
+ ((S_ISCHR(mode) || S_ISBLK(mode)) && !capable(CAP_MKNOD)))
+ return -EPERM;
+
+ req = fuse_get_req(fc);
if (IS_ERR(req))
return PTR_ERR(req);