Re: [NFS] [PATCH] Bug in NFS - should init be allowed to set umask???

Neil Brown (neilb@cse.unsw.edu.au)
Sat, 14 Jul 2001 08:47:40 +1000 (EST)


On Friday July 13, neilb@cse.unsw.edu.au wrote:
> So we have several options:
>
> 1/ Claim that redhat is broken. Leave them to fix SysVinit.
> 2/ Have nfsd over-write the umask setting that /sbin/init imposed.
> This is effectively what your patch does.
> 3/ Decide that it is inappropriate for nfsd to share the current->fs
> fs_struct with init. Unfortunately this means changing or
> replacing daemonize().
>
> None of these seem ideal. (3) is probably most correct (i.e. protect
> the kernel from user space mucking about) but is the most work.

I've found a 4th option. We make it so that fs->umask does not affect
nfsd, just as the rest of the fs_struct has no effect on nfsd.
This means moving the " & ~current->fs->umask" up out of the vfs_*
routines and into the sys_* (or open_namei) routines.

The attached patch does this.
The only part that I'm not 100% confident of is the call to vfs_mknod
in net/unix/af_unix.c:unix_bind. I haven't put an "& ~current->fs->umask"
there, as I suspect that is actually the right thing. Any comments on
that?

Al, as this is a vfs change, I have cc:ed you. Are you happy with it?
It allows nfsd to not be effected by any change the init might make to
umask.

This patch also puts the umask for init back to 0022.

NeilBrown

--- ./fs/namei.c 2001/07/13 21:43:03 1.1
+++ ./fs/namei.c 2001/07/13 22:38:31 1.2
@@ -886,7 +886,7 @@
{
int error;

- mode &= S_IALLUGO & ~current->fs->umask;
+ mode &= S_IALLUGO;
mode |= S_IFREG;

down(&dir->i_zombie);
@@ -975,7 +975,8 @@

/* Negative dentry, just create the file */
if (!dentry->d_inode) {
- error = vfs_create(dir->d_inode, dentry, mode);
+ error = vfs_create(dir->d_inode, dentry,
+ mode & ~current->fs->umask);
up(&dir->d_inode->i_sem);
dput(nd->dentry);
nd->dentry = dentry;
@@ -1164,8 +1165,6 @@
{
int error = -EPERM;

- mode &= ~current->fs->umask;
-
down(&dir->i_zombie);
if ((S_ISCHR(mode) || S_ISBLK(mode)) && !capable(CAP_MKNOD))
goto exit_lock;
@@ -1208,6 +1207,8 @@
goto out;
dentry = lookup_create(&nd, 0);
error = PTR_ERR(dentry);
+
+ mode &= ~current->fs->umask;
if (!IS_ERR(dentry)) {
switch (mode & S_IFMT) {
case 0: case S_IFREG:
@@ -1246,7 +1247,7 @@
goto exit_lock;

DQUOT_INIT(dir);
- mode &= (S_IRWXUGO|S_ISVTX) & ~current->fs->umask;
+ mode &= (S_IRWXUGO|S_ISVTX);
lock_kernel();
error = dir->i_op->mkdir(dir, dentry, mode);
unlock_kernel();
@@ -1276,7 +1277,8 @@
dentry = lookup_create(&nd, 1);
error = PTR_ERR(dentry);
if (!IS_ERR(dentry)) {
- error = vfs_mkdir(nd.dentry->d_inode, dentry, mode);
+ error = vfs_mkdir(nd.dentry->d_inode, dentry,
+ mode & ~current->fs->umask);
dput(dentry);
}
up(&nd.dentry->d_inode->i_sem);

--- ./include/linux/fs_struct.h 2001/07/13 22:46:19 1.1
+++ ./include/linux/fs_struct.h 2001/07/13 22:46:40 1.2
@@ -13,7 +13,7 @@
#define INIT_FS { \
ATOMIC_INIT(1), \
RW_LOCK_UNLOCKED, \
- 0000, \
+ 0022, \
NULL, NULL, NULL, NULL, NULL, NULL \
}

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/