Messages in this thread Patch in this message | | | Date | Mon, 22 Jun 1998 17:45:22 +0400 | From | Savochkin Andrey Vladimirovich <> | Subject | [patch] Re: mount(2) bug found |
| |
On Tue, Jun 16, 1998 at 09:21:26AM +0200, Rik van Riel wrote: > On Tue, 16 Jun 1998, Chris Wedgwood wrote: > > On Mon, Jun 15, 1998 at 04:38:33PM +0200, Rik van Riel wrote: > > > > > > The directory gets mounted correctly, however the status > > > in /proc/mounts is _not_ displayed correctly. After I've > > > installed Stampede, I'll investigate the bug and correct > > > it for both 2.0 and 2.1 kernels... > > > > 'Thats not a bug - its a feature' <tm>. > > It _is_ a bug. Just look at the following scenario: > > mirkwood:# df > /dev/root / <blahblahblah> > /dev/sda4 /stampede <blahblahblah> > mirkwood:# chroot /stampede /bin/bash > chroot:# mount /dev/sda1 /mnt > chroot:# mount -t proc none /proc > <flip to other console> > mirkwood:# cat /proc/mounts > /dev/root / <blah> > /dev/sda4 /stampede <blah> > none /proc <blah> > /dev/sda1 /mnt <blah> <-- mounted on /stampede/mnt > none /proc <blah> <-- mounted on /stampede/proc > > So you see, it _is_ a bug... This is mainly because > programs like 'df' depend on the fact that the FSes > are correctly listed. > The above setup clearly breaks 'df' and other programs. > > Furthermore, it would be nice to have chroot() in such > a way that you can't see you're chroot()ed... > The security buffs would probably like such a setup > too :-)
Hi,
I like chroot()'ed environment and prefer correct /proc/mounts. During the weekend I've implemented and slightly tested a patch fixing the problem for 2.1 kernels.
More precisely, the patch does the following. 1. Entries in /proc/mounts are produced using dentries of the mount point. They are independent of the root of the mount process and remained correct after directory rename etc. 2. Entries in /proc/mounts are relative to the root of reading process. A process can't see mount points lying outside its root. 3. The limit of pagesize for the entire /proc/mounts contents was removed. The only limit remains is a pagesize limit for a line. 4. Broken boundary check before sprintf was eliminated. The previous implementation allows long mount lines corrupt the kernel memory.
Best regards Andrey V. Savochkin --- linux.orig/fs/super.c Sat May 9 15:35:06 1998 +++ linux/fs/super.c Mon Jun 22 14:59:30 1998 @@ -314,18 +314,16 @@ { 0, NULL } }; -int get_filesystem_info( char *buf ) +static size_t get_filesystem_info(struct vfsmount *tmp, char *buf) { - struct vfsmount *tmp = vfsmntlist; struct proc_fs_info *fs_infop; struct proc_nfs_info *nfs_infop; struct nfs_server *nfss; int len = 0; - while ( tmp && len < PAGE_SIZE - 160) { - len += sprintf( buf + len, "%s %s %s %s", - tmp->mnt_devname, tmp->mnt_dirname, tmp->mnt_sb->s_type->name, + len += sprintf( buf + len, " %s %s", + tmp->mnt_sb->s_type->name, tmp->mnt_flags & MS_RDONLY ? "ro" : "rw" ); for (fs_infop = fs_info; fs_infop->flag; fs_infop++) { if (tmp->mnt_flags & fs_infop->flag) { @@ -379,10 +377,88 @@ nfss->hostname); } len += sprintf( buf + len, " 0 0\n" ); - tmp = tmp->mnt_next; } return len; +} + +static size_t get_mtab_line(struct vfsmount *tmp, char *buf) +{ + char *mntpnt; + int slen1, slen2; + int is_nfs; + size_t len; + + /* get relative mount point */ + mntpnt = d_rel_path(tmp->mnt_sb->s_root, buf, PAGE_SIZE); + if (mntpnt == NULL) + return 0; + slen1 = strlen(tmp->mnt_devname); + slen2 = strlen(mntpnt); + is_nfs = !strcmp("nfs", tmp->mnt_sb->s_type->name); + if (slen1 + slen2 + + strlen(tmp->mnt_sb->s_type->name) + + (is_nfs ? + strlen(tmp->mnt_sb->u.nfs_sb.s_server.hostname) + 512 : + 256) + > PAGE_SIZE) + return 0; + + memmove(buf + slen1 + 1, mntpnt, slen2); + memcpy(buf, tmp->mnt_devname, slen1); + buf[slen1] = ' '; + len = slen1 + 1 + slen2; + len += get_filesystem_info(tmp, buf + len); + + return len; +} + +ssize_t read_mtab(struct file *file, char *usrbuf, size_t count, loff_t *ppos) +{ + ssize_t retval; + char *buf; + struct vfsmount *tmp; + loff_t ourpos; + size_t len, l, off; + + retval = -ENOMEM; + buf = (char*)__get_free_page(GFP_KERNEL); + if (!buf) + goto out; + + retval = 0; + ourpos = 0; + for (tmp = vfsmntlist; tmp != NULL; tmp = tmp->mnt_next) { + if (ourpos >= *ppos + count) + break; + + len = get_mtab_line(tmp, buf); + if (!len) + continue; + + /* check if [ourpos, ourpos+len) + overlaps [*ppos, *ppos+count) */ + if (*ppos < ourpos + len) { + /* copy to user's buffer */ + if (*ppos > ourpos) + off = *ppos - ourpos; + else + off = 0; + l = (*ppos + count) - (ourpos + off); + if (l > len - off) + l = len - off; + *ppos += l; + copy_to_user(usrbuf, buf + off, l); + usrbuf += l; + count -= l; + retval += l; + } + ourpos += len; + } + + free_page((unsigned long)buf); +out: + return retval; } int get_filesystem_list(char * buf) --- linux.orig/fs/proc/array.c Wed Jun 17 14:47:03 1998 +++ linux/fs/proc/array.c Sat Jun 20 20:49:01 1998 @@ -1200,7 +1200,6 @@ #endif extern int get_device_list(char *); extern int get_filesystem_list(char *); -extern int get_filesystem_info( char * ); extern int get_irq_list(char *); extern int get_dma_list(char *); extern int get_cpuinfo(char *); @@ -1277,9 +1276,6 @@ case PROC_CMDLINE: return get_cmdline(page); - case PROC_MTAB: - return get_filesystem_info( page ); - case PROC_SWAP: return get_swaparea_info(page); #ifdef CONFIG_RTC @@ -1422,13 +1418,21 @@ static ssize_t arraylong_read(struct file * file, char * buf, size_t count, loff_t *ppos) { + extern ssize_t read_mtab(struct file *, char *, size_t, loff_t *); struct inode * inode = file->f_dentry->d_inode; unsigned int pid = inode->i_ino >> 16; unsigned int type = inode->i_ino & 0x0000ffff; - switch (type) { - case PROC_PID_MAPS: - return read_maps(pid, file, buf, count, ppos); + if (pid) { + switch (type) { + case PROC_PID_MAPS: + return read_maps(pid, file, buf, count, ppos); + } + }else { + switch (type) { + case PROC_MTAB: + return read_mtab(file, buf, count, ppos); + } } return -EINVAL; } --- linux.orig/fs/proc/root.c Fri Jun 5 14:23:43 1998 +++ linux/fs/proc/root.c Fri Jun 19 19:45:48 1998 @@ -604,7 +604,7 @@ static struct proc_dir_entry proc_root_mounts = { PROC_MTAB, 6, "mounts", S_IFREG | S_IRUGO, 1, 0, 0, - 0, &proc_array_inode_operations + 0, &proc_arraylong_inode_operations }; static struct proc_dir_entry proc_root_swaps = { PROC_SWAP, 5, "swaps", --- linux.orig/fs/dcache.c Fri May 8 12:46:03 1998 +++ linux/fs/dcache.c Mon Jun 22 17:26:41 1998 @@ -696,21 +696,14 @@ /* * "buflen" should be PAGE_SIZE or more. + * The search stops at / or current->fs->root. */ -char * d_path(struct dentry *dentry, char *buffer, int buflen) +static char * _d_path(struct dentry **d, char *end, int buflen) { - char * end = buffer+buflen; char * retval; + struct dentry * dentry = *d; struct dentry * root = current->fs->root; - *--end = '\0'; - buflen--; - if (dentry->d_parent != dentry && list_empty(&dentry->d_hash)) { - buflen -= 10; - end -= 10; - memcpy(end, " (deleted)", 10); - } - /* Get '/' right */ retval = end-1; *retval = '/'; @@ -735,6 +728,41 @@ retval = end; dentry = parent; } + *d = dentry; + return retval; +} + +char * d_rel_path(struct dentry *dentry, char *buffer, int buflen) +{ + char * end = buffer+buflen; + char * retval; + + *--end = '\0'; + buflen--; + + retval = _d_path(&dentry, end, buflen); + if (dentry != current->fs->root) + retval = NULL; + return retval; +} + +/* + * "buflen" should be PAGE_SIZE or more. + */ +char * d_path(struct dentry *dentry, char *buffer, int buflen) +{ + char * end = buffer+buflen; + char * retval; + + *--end = '\0'; + buflen--; + if (dentry->d_parent != dentry && list_empty(&dentry->d_hash)) { + buflen -= 10; + end -= 10; + memcpy(end, " (deleted)", 10); + } + + retval = _d_path(&dentry, end, buflen); return retval; } --- linux.orig/include/linux/dcache.h Fri May 8 12:46:13 1998 +++ linux/include/linux/dcache.h Fri Jun 19 21:42:29 1998 @@ -157,6 +157,13 @@ /* write full pathname into buffer and return start of pathname */ extern char * d_path(struct dentry * entry, char * buf, int buflen); +/* + * write pathname relatively the root of process; + * return: start of pathname + * NULL if pathname isn't inside root or on other problems + */ +extern char * d_rel_path(struct dentry * entry, char * buf, int buflen); + /* Allocation counts.. */ static __inline__ struct dentry * dget(struct dentry *dentry) { | |