Messages in this thread Patch in this message | | | Date | Sun, 16 May 1999 02:26:44 -0400 (EDT) | From | Alexander Viro <> | Subject | [FIX] Re: Security bug: mkdir removes the sticky bit |
| |
On Fri, 14 May 1999, Wolfram Kleff wrote:
> Security bug: mkdir removes the sticky bit > > There is a security bug in mkdir: > > > mkdir -m 1777 /tmp/foo > > ls -l /tmp > drwxrwxrwx 2 root root 1024 May 14 19:44 foo > > The sticky bit is missing. > > > strace mkdir -m 1777 /tmp/foo |& grep mkdir > mkdir("/tmp/foo", 01777) = 0 > > So it looks to be really kernel related.
Yup, it is. Patch follows:
diff -urN linux-2.3.2/fs/ChangeLog linux-bird.mkdir/fs/ChangeLog --- linux-2.3.2/fs/ChangeLog Thu May 13 07:02:54 1999 +++ linux-bird.mkdir/fs/ChangeLog Sun May 16 02:33:54 1999 @@ -144,24 +144,12 @@ * rmdir of immutable/append-only directory shouldn't be allowed. Fixed. Remains unfixed: - * UMSDOS_rename is broken. Call it with the dest. existing and being an - empty directory and you've got EBUSY. At least it doesn't do - any harm, so that will wait several days till rename cleanup. - Sigh... It will wait a bit more. Problems with fat-derived - filesystems are much worse than I thought. Idea of changing - inode under dentry is broken by design - guess where the - semaphore sits, for one. - * umsdos: weird. rename() shouldn't return -EEXIST. BTW, manpage - for rename(2) is obviously bogus - it mentions EEXIST and - on the next line (correctly) says that EINVAL should be - returned. Under the same conditions. * rename's handling of races is, erm, not optimal. Looks like I know what to do, but this thing needs some more cleanup - we can take care of almost all races in VFS and be much more graceful wrt locking. Moreover, it would give strong lookup atomicity. But it's a lot of changes to lookup and dcache code, so it will go after the fs drivers' cleanup. - * hfs allows mknod. Only for regular files ;-/ IMHO it's bogus. * affs allows HARD links to directories. VFS is, to put it politely, not too ready to cope with _that_. And I'm not sure it should be - looks like they are pretty much similar to symlinks. @@ -169,8 +157,3 @@ braindead filesystems). I've submitted a patch to Linus, but looks like it wasn't applied. * msdos: shouldn't we treat SYS as IMMUTABLE? Makes sense, IMHO. - * minix, qnx and sysv do NOT allow to mkdir sticky directories. - * {minix,sysv}/namei.c (do_{minix,syv}_{rename,unlink}): - Stuff related to retries still needs cleanup/fixing. - Looks like I've found an extremely low-probability race - there... diff -urN linux-2.3.2/fs/affs/namei.c linux-bird.mkdir/fs/affs/namei.c --- linux-2.3.2/fs/affs/namei.c Thu May 13 07:02:55 1999 +++ linux-bird.mkdir/fs/affs/namei.c Sun May 16 02:30:04 1999 @@ -315,7 +315,7 @@ error = affs_add_entry(dir,NULL,inode,dentry,ST_USERDIR); if (error) goto out_iput; - inode->i_mode = S_IFDIR | S_ISVTX | (mode & 0777 & ~current->fs->umask); + inode->i_mode = S_IFDIR | S_ISVTX | mode; inode->u.affs_i.i_protect = mode_to_prot(inode->i_mode); d_instantiate(dentry,inode); mark_inode_dirty(inode); diff -urN linux-2.3.2/fs/ext2/namei.c linux-bird.mkdir/fs/ext2/namei.c --- linux-2.3.2/fs/ext2/namei.c Thu May 13 23:03:17 1999 +++ linux-bird.mkdir/fs/ext2/namei.c Sun May 16 02:27:54 1999 @@ -490,7 +490,7 @@ inode->i_nlink = 2; mark_buffer_dirty(dir_block, 1); brelse (dir_block); - inode->i_mode = S_IFDIR | (mode & (S_IRWXUGO|S_ISVTX) & ~current->fs->umask); + inode->i_mode = S_IFDIR | mode; if (dir->i_mode & S_ISGID) inode->i_mode |= S_ISGID; mark_inode_dirty(inode); diff -urN linux-2.3.2/fs/minix/namei.c linux-bird.mkdir/fs/minix/namei.c --- linux-2.3.2/fs/minix/namei.c Thu May 13 23:03:18 1999 +++ linux-bird.mkdir/fs/minix/namei.c Sun May 16 02:25:31 1999 @@ -297,7 +297,7 @@ inode->i_nlink = 2; mark_buffer_dirty(dir_block, 1); brelse(dir_block); - inode->i_mode = S_IFDIR | (mode & 0777 & ~current->fs->umask); + inode->i_mode = S_IFDIR | mode; if (dir->i_mode & S_ISGID) inode->i_mode |= S_ISGID; mark_inode_dirty(inode); diff -urN linux-2.3.2/fs/namei.c linux-bird.mkdir/fs/namei.c --- linux-2.3.2/fs/namei.c Thu May 13 23:03:18 1999 +++ linux-bird.mkdir/fs/namei.c Sun May 16 02:19:42 1999 @@ -882,10 +882,6 @@ return error; } -/* - * Look out: this function may change a normal dentry - * into a directory dentry (different size).. - */ static inline int do_mkdir(const char * pathname, int mode) { int error; @@ -919,7 +915,7 @@ goto exit_lock; DQUOT_INIT(dir->d_inode); - mode &= 0777 & ~current->fs->umask; + mode &= (S_IRWXUGO|S_ISVTX) & ~current->fs->umask; error = dir->d_inode->i_op->mkdir(dir->d_inode, dentry, mode); exit_lock: diff -urN linux-2.3.2/fs/nfsd/vfs.c linux-bird.mkdir/fs/nfsd/vfs.c --- linux-2.3.2/fs/nfsd/vfs.c Thu May 13 07:03:02 1999 +++ linux-bird.mkdir/fs/nfsd/vfs.c Sun May 16 02:19:37 1999 @@ -691,6 +691,8 @@ break; case S_IFDIR: opfunc = (nfsd_dirop_t) dirp->i_op->mkdir; + /* Odd, indeed, but filesystems did it anyway */ + iap->ia_mode &= (S_IRWXUGO|S_ISVTX) & ~current->fs->umask; break; case S_IFCHR: case S_IFBLK: diff -urN linux-2.3.2/fs/sysv/namei.c linux-bird.mkdir/fs/sysv/namei.c --- linux-2.3.2/fs/sysv/namei.c Thu May 13 23:03:19 1999 +++ linux-bird.mkdir/fs/sysv/namei.c Sun May 16 02:21:07 1999 @@ -291,7 +291,7 @@ inode->i_nlink = 2; mark_buffer_dirty(dir_block, 1); brelse(dir_block); - inode->i_mode = S_IFDIR | (mode & 0777 & ~current->fs->umask); + inode->i_mode = S_IFDIR | mode; if (dir->i_mode & S_ISGID) inode->i_mode |= S_ISGID; mark_inode_dirty(inode); diff -urN linux-2.3.2/fs/ufs/namei.c linux-bird.mkdir/fs/ufs/namei.c --- linux-2.3.2/fs/ufs/namei.c Thu May 13 23:03:19 1999 +++ linux-bird.mkdir/fs/ufs/namei.c Sun May 16 02:22:02 1999 @@ -539,7 +539,7 @@ inode->i_nlink = 2; mark_buffer_dirty(dir_block, 1); brelse (dir_block); - inode->i_mode = S_IFDIR | (mode & (S_IRWXUGO|S_ISVTX) & ~current->fs->umask); + inode->i_mode = S_IFDIR | mode; if (dir->i_mode & S_ISGID) inode->i_mode |= S_ISGID; mark_inode_dirty(inode);
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/
| |