lkml.org 
[lkml]   [1999]   [Apr]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patches in this message
/
Date
From
SubjectRe: [PATCH] missing lock_kernel() and assorted races


On Fri, 30 Apr 1999, Alan Cox wrote:

> > Linus, we have several problems under arch/:
> >
> > The architectures where you made the most changes don't even compile
> > in 2.2.7 and haven't been updated in nearly half of a year :-)
>
> And some of the bugs appear to be in the relevant CVS trees too - although
> the mips doesnt do SMP so its harmless

Harmless it might be, if it would be restricted to missing lock_kernel();-/
sysirix.c in both trees has UP-affecting bugs. That is, irix_fstatvfs()
and irix_fstatvfs64() are oopsable; irix_ngetdents(), irix_getdents64()
and irix_ngetdents64() do not protect ->readdir() with semaphore. Those
bugs are present both in 1.16 (in mips tree) and 1.12 (in main tree).
I'm submitting the patch to Ralf and I've merged relevant changes into the
patch against the main tree. Patches attached.
Cheers,
Al
--- sysirix.c.old Sat Feb 20 20:40:33 1999
+++ sysirix.c Fri Apr 30 05:19:51 1999
@@ -736,6 +736,7 @@
int error, i;

/* We don't support this feature yet. */
+ lock_kernel();
if(fs_type) {
error = -EINVAL;
goto out;
@@ -824,7 +825,6 @@
__put_user(0, &buf->f_fname[i]);
__put_user(0, &buf->f_fpack[i]);
}
- error = 0;

out_f:
fput(file);
@@ -1590,6 +1590,10 @@
error = -ENOENT;
goto out_f;
}
+ if (!inode->i_sb) {
+ error = -ENODEV;
+ goto out_f;
+ }
if (!inode->i_sb->s_op->statfs) {
error = -ENOSYS;
goto out_f;
@@ -1622,8 +1626,6 @@
for(i = 0; i < 32; i++)
__put_user(0, &buf->f_fstr[i]);

- error = 0;
-
out_f:
fput(file);
out:
@@ -1809,6 +1811,7 @@
struct statfs kbuf;
int error, i;

+ lock_kernel();
printk("[%s:%ld] Wheee.. irix_statvfs(%s,%p)\n",
current->comm, current->pid, fname, buf);
error = verify_area(VERIFY_WRITE, buf, sizeof(struct irix_statvfs));
@@ -1882,6 +1885,10 @@
error = -ENOENT;
goto out_f;
}
+ if (!inode->i_sb) {
+ error = -ENODEV;
+ goto out_f;
+ }
if (!inode->i_sb->s_op->statfs) {
error = -ENOSYS;
goto out_f;
@@ -1913,9 +1920,6 @@
__put_user(kbuf.f_namelen, &buf->f_namemax);
for(i = 0; i < 32; i++)
__put_user(0, &buf->f_fstr[i]);
-
- error = 0;
-
out_f:
fput(file);
out:
@@ -1987,7 +1991,6 @@
unsigned short reclen = ROUND_UP32(NAME_OFFSET32(dirent) + namlen + 1);
int retval;

- lock_kernel();
#ifdef DEBUG_GETDENTS
printk("\nirix_filldir32[reclen<%d>namlen<%d>count<%d>]",
reclen, namlen, buf->count);
@@ -2013,14 +2016,12 @@
retval = 0;

out:
- unlock_kernel();
return retval;
}

asmlinkage int irix_ngetdents(unsigned int fd, void * dirent, unsigned int count, int *eob)
{
struct file *file;
- struct dentry *dentry;
struct inode *inode;
struct irix_dirent32 *lastdirent;
struct irix_dirent32_callback buf;
@@ -2036,8 +2037,8 @@
if (!file)
goto out;

- dentry = file->f_dentry;
- if (!dentry)
+ inode = file->f_dentry->d_inode;
+ if (!inode)
goto out_putf;

inode = dentry->d_inode;
@@ -2113,7 +2114,6 @@
unsigned short reclen = ROUND_UP64(NAME_OFFSET64(dirent) + namlen + 1);
int retval;

- lock_kernel();
buf->error = -EINVAL; /* only used if we fail.. */
if (reclen > buf->count) {
retval = -EINVAL;
@@ -2134,7 +2134,6 @@

retval = 0;
out:
- unlock_kernel();
return retval;
}

@@ -2175,7 +2174,9 @@
buf.previous = NULL;
buf.count = cnt;
buf.error = 0;
+ down(&inode->i_sem);
error = file->f_op->readdir(file, &buf, irix_filldir64);
+ up(&inode->i_sem);
if (error < 0)
goto out_f;
lastdirent = buf.previous;
@@ -2235,7 +2236,9 @@
buf.previous = NULL;
buf.count = cnt;
buf.error = 0;
+ down(&inode->i_sem);
error = file->f_op->readdir(file, &buf, irix_filldir64);
+ up(&inode->i_sem);
if (error < 0)
goto out_f;
lastdirent = buf.previous;diff -urN linux-2.2.7/arch/mips/kernel/sysirix.c linux-bird.arch/arch/mips/kernel/sysirix.c
--- linux-2.2.7/arch/mips/kernel/sysirix.c Sun Mar 28 14:45:33 1999
+++ linux-bird.arch/arch/mips/kernel/sysirix.c Thu Apr 29 22:59:07 1999
@@ -23,6 +23,7 @@
#include <linux/smp.h>
#include <linux/smp_lock.h>
#include <linux/utsname.h>
+#include <linux/file.h>

#include <asm/ptrace.h>
#include <asm/page.h>
@@ -734,6 +735,7 @@
int error, i;

/* We don't support this feature yet. */
+ lock_kernel();
if(fs_type) {
error = -EINVAL;
goto out;
@@ -776,7 +778,6 @@

asmlinkage int irix_fstatfs(unsigned int fd, struct irix_statfs *buf)
{
- struct dentry *dentry;
struct inode *inode;
struct statfs kbuf;
mm_segment_t old_fs;
@@ -787,25 +788,22 @@
error = verify_area(VERIFY_WRITE, buf, sizeof(struct irix_statfs));
if (error)
goto out;
- if (fd >= NR_OPEN || !(file = current->files->fd[fd])) {
+ if (!(file = fget(fd))) {
error = -EBADF;
goto out;
}
- if (!(dentry = file->f_dentry)) {
+
+ if (!(inode = file->f_dentry->d_inode)) {
error = -ENOENT;
- goto out;
- }
- if (!(inode = dentry->d_inode)) {
- error = -ENOENT;
- goto out;
+ goto out_f;
}
if (!inode->i_sb) {
error = -ENODEV;
- goto out;
+ goto out_f;
}
if (!inode->i_sb->s_op->statfs) {
error = -ENOSYS;
- goto out;
+ goto out_f;
}

old_fs = get_fs(); set_fs(get_ds());
@@ -813,7 +811,7 @@
sizeof(struct statfs));
set_fs(old_fs);
if (error)
- goto out;
+ goto out_f;

__put_user(kbuf.f_type, &buf->f_type);
__put_user(kbuf.f_bsize, &buf->f_bsize);
@@ -826,9 +824,9 @@
__put_user(0, &buf->f_fname[i]);
__put_user(0, &buf->f_fpack[i]);
}
- error = 0;

- dput(dentry);
+out_f:
+ fput(file);
out:
unlock_kernel();
return error;
@@ -1110,7 +1108,7 @@

lock_kernel();
if(!(flags & MAP_ANONYMOUS)) {
- if(fd >= NR_OPEN || !(file = current->files->fd[fd])) {
+ if(!(file = fget(fd))) {
retval = -EBADF;
goto out;
}
@@ -1130,6 +1128,8 @@
flags &= ~(MAP_EXECUTABLE | MAP_DENYWRITE);

retval = do_mmap(file, addr, len, prot, flags, offset);
+ if (file)
+ fput(file);

out:
unlock_kernel();
@@ -1568,7 +1568,6 @@

asmlinkage int irix_fstatvfs(int fd, struct irix_statvfs *buf)
{
- struct dentry *dentry;
struct inode *inode;
mm_segment_t old_fs;
struct statfs kbuf;
@@ -1582,21 +1581,21 @@
error = verify_area(VERIFY_WRITE, buf, sizeof(struct irix_statvfs));
if (error)
goto out;
- if (fd >= NR_OPEN || !(file = current->files->fd[fd])) {
+ if (!(file = fget(fd))) {
error = -EBADF;
goto out;
}
- if (!(dentry = file->f_dentry)) {
+ if (!(inode = file->f_dentry->d_inode)) {
error = -ENOENT;
- goto out;
+ goto out_f;
}
- if (!(inode = dentry->d_inode)) {
- error = -ENOENT;
- goto out;
+ if (!inode->i_sb) {
+ error = -ENODEV;
+ goto out_f;
}
if (!inode->i_sb->s_op->statfs) {
error = -ENOSYS;
- goto out;
+ goto out_f;
}

old_fs = get_fs(); set_fs(get_ds());
@@ -1604,7 +1603,7 @@
sizeof(struct statfs));
set_fs(old_fs);
if (error)
- goto out;
+ goto out_f;

__put_user(kbuf.f_bsize, &buf->f_bsize);
__put_user(kbuf.f_frsize, &buf->f_frsize);
@@ -1626,9 +1625,8 @@
for(i = 0; i < 32; i++)
__put_user(0, &buf->f_fstr[i]);

- error = 0;
-
- dput(dentry);
+out_f:
+ fput(file);
out:
unlock_kernel();
return error;
@@ -1726,7 +1724,7 @@
}

if(!(flags & MAP_ANONYMOUS)) {
- if(fd >= NR_OPEN || !(file = current->files->fd[fd])) {
+ if(!(file = fcheck(fd))) {
error = -EBADF;
goto out;
}
@@ -1812,6 +1810,7 @@
struct statfs kbuf;
int error, i;

+ lock_kernel();
printk("[%s:%ld] Wheee.. irix_statvfs(%s,%p)\n",
current->comm, current->pid, fname, buf);
error = verify_area(VERIFY_WRITE, buf, sizeof(struct irix_statvfs));
@@ -1864,7 +1863,6 @@

asmlinkage int irix_fstatvfs64(int fd, struct irix_statvfs *buf)
{
- struct dentry *dentry;
struct inode *inode;
mm_segment_t old_fs;
struct statfs kbuf;
@@ -1878,21 +1876,21 @@
error = verify_area(VERIFY_WRITE, buf, sizeof(struct irix_statvfs));
if (error)
goto out;
- if (fd >= NR_OPEN || !(file = current->files->fd[fd])) {
+ if (!(file = fget(fd))) {
error = -EBADF;
goto out;
}
- if (!(dentry = file->f_dentry)) {
+ if (!(inode = file->f_dentry->d_inode)) {
error = -ENOENT;
- goto out;
+ goto out_f;
}
- if (!(inode = dentry->d_inode)) {
- error = -ENOENT;
- goto out;
+ if (!inode->i_sb) {
+ error = -ENODEV;
+ goto out_f;
}
if (!inode->i_sb->s_op->statfs) {
error = -ENOSYS;
- goto out;
+ goto out_f;
}

old_fs = get_fs(); set_fs(get_ds());
@@ -1900,7 +1898,7 @@
sizeof(struct statfs));
set_fs(old_fs);
if (error)
- goto out;
+ goto out_f;

__put_user(kbuf.f_bsize, &buf->f_bsize);
__put_user(kbuf.f_frsize, &buf->f_frsize);
@@ -1921,10 +1919,8 @@
__put_user(kbuf.f_namelen, &buf->f_namemax);
for(i = 0; i < 32; i++)
__put_user(0, &buf->f_fstr[i]);
-
- error = 0;
-
- dput(dentry);
+out_f:
+ fput(file);
out:
unlock_kernel();
return error;
@@ -1994,7 +1990,6 @@
unsigned short reclen = ROUND_UP32(NAME_OFFSET32(dirent) + namlen + 1);
int retval;

- lock_kernel();
#ifdef DEBUG_GETDENTS
printk("\nirix_filldir32[reclen<%d>namlen<%d>count<%d>]",
reclen, namlen, buf->count);
@@ -2020,14 +2015,12 @@
retval = 0;

out:
- unlock_kernel();
return retval;
}

asmlinkage int irix_ngetdents(unsigned int fd, void * dirent, unsigned int count, int *eob)
{
struct file *file;
- struct dentry *dentry;
struct inode *inode;
struct irix_dirent32 *lastdirent;
struct irix_dirent32_callback buf;
@@ -2039,46 +2032,56 @@
current->pid, fd, dirent, count, eob);
#endif
error = -EBADF;
- if (fd >= NR_OPEN || !(file = current->files->fd[fd]))
+ file = fget(fd);
+ if (!file)
goto out;

- dentry = file->f_dentry;
- if (!dentry)
- goto out;
+ inode = file->f_dentry->d_inode;
+ if (!inode)
+ goto out_putf;

inode = dentry->d_inode;
if (!inode)
- goto out;
+ goto out_putf;

- error = -ENOTDIR;
- if (!file->f_op || !file->f_op->readdir)
- goto out;
-
- error = -EFAULT;
- if(!access_ok(VERIFY_WRITE, dirent, count) ||
- !access_ok(VERIFY_WRITE, eob, sizeof(*eob)))
- goto out;
-
- __put_user(0, eob);
buf.current_dir = (struct irix_dirent32 *) dirent;
buf.previous = NULL;
buf.count = count;
buf.error = 0;

+ error = -ENOTDIR;
+ if (!file->f_op || !file->f_op->readdir)
+ goto out_putf;
+
+ /*
+ * Get the inode's semaphore to prevent changes
+ * to the directory while we read it.
+ */
+ down(&inode->i_sem);
error = file->f_op->readdir(file, &buf, irix_filldir32);
+ up(&inode->i_sem);
if (error < 0)
- goto out;
+ goto out_putf;
+ error = buf.error;
lastdirent = buf.previous;
- if (!lastdirent) {
- error = buf.error;
- goto out;
+ if (lastdirent) {
+ put_user(file->f_pos, &lastdirent->d_off);
+ error = count - buf.count;
}
- lastdirent->d_off = (u32) file->f_pos;
+
+ if (put_user(0, eob) < 0) {
+ error = EFAULT;
+ goto out_putf;
+ }
+
+
#ifdef DEBUG_GETDENTS
printk("eob=%d returning %d\n", *eob, count - buf.count);
#endif
error = count - buf.count;

+out_putf:
+ fput(file);
out:
unlock_kernel();
return error;
@@ -2110,7 +2113,6 @@
unsigned short reclen = ROUND_UP64(NAME_OFFSET64(dirent) + namlen + 1);
int retval;

- lock_kernel();
buf->error = -EINVAL; /* only used if we fail.. */
if (reclen > buf->count) {
retval = -EINVAL;
@@ -2131,14 +2133,12 @@

retval = 0;
out:
- unlock_kernel();
return retval;
}

asmlinkage int irix_getdents64(int fd, void *dirent, int cnt)
{
struct file *file;
- struct dentry *dentry;
struct inode *inode;
struct irix_dirent64 *lastdirent;
struct irix_dirent64_callback buf;
@@ -2150,40 +2150,38 @@
current->pid, fd, dirent, cnt);
#endif
error = -EBADF;
- if (fd >= NR_OPEN || !(file = current->files->fd[fd]))
- goto out;
-
- dentry = file->f_dentry;
- if (!dentry)
+ if (!(file = fget(fd)))
goto out;

- inode = dentry->d_inode;
+ inode = file->f_dentry->d_inode;
if (!inode)
- goto out;
+ goto out_f;

error = -ENOTDIR;
if (!file->f_op || !file->f_op->readdir)
- goto out;
+ goto out_f;

error = -EFAULT;
if(!access_ok(VERIFY_WRITE, dirent, cnt))
- goto out;
+ goto out_f;

error = -EINVAL;
if(cnt < (sizeof(struct irix_dirent64) + 255))
- goto out;
+ goto out_f;

buf.curr = (struct irix_dirent64 *) dirent;
buf.previous = NULL;
buf.count = cnt;
buf.error = 0;
+ down(&inode->i_sem);
error = file->f_op->readdir(file, &buf, irix_filldir64);
+ up(&inode->i_sem);
if (error < 0)
- goto out;
+ goto out_f;
lastdirent = buf.previous;
if (!lastdirent) {
error = buf.error;
- goto out;
+ goto out_f;
}
lastdirent->d_off = (u64) file->f_pos;
#ifdef DEBUG_GETDENTS
@@ -2191,6 +2189,8 @@
#endif
error = cnt - buf.count;

+out_f:
+ fput(file);
out:
unlock_kernel();
return error;
@@ -2199,7 +2199,6 @@
asmlinkage int irix_ngetdents64(int fd, void *dirent, int cnt, int *eob)
{
struct file *file;
- struct dentry *dentry;
struct inode *inode;
struct irix_dirent64 *lastdirent;
struct irix_dirent64_callback buf;
@@ -2211,42 +2210,40 @@
current->pid, fd, dirent, cnt);
#endif
error = -EBADF;
- if (fd >= NR_OPEN || !(file = current->files->fd[fd]))
- goto out;
-
- dentry = file->f_dentry;
- if (!dentry)
+ if (!(file = fget(fd)))
goto out;

- inode = dentry->d_inode;
+ inode = file->f_dentry->d_inode;
if (!inode)
- goto out;
+ goto out_f;

error = -ENOTDIR;
if (!file->f_op || !file->f_op->readdir)
- goto out;
+ goto out_f;

error = -EFAULT;
if(!access_ok(VERIFY_WRITE, dirent, cnt) ||
!access_ok(VERIFY_WRITE, eob, sizeof(*eob)))
- goto out;
+ goto out_f;

error = -EINVAL;
if(cnt < (sizeof(struct irix_dirent64) + 255))
- goto out;
+ goto out_f;

*eob = 0;
buf.curr = (struct irix_dirent64 *) dirent;
buf.previous = NULL;
buf.count = cnt;
buf.error = 0;
+ down(&inode->i_sem);
error = file->f_op->readdir(file, &buf, irix_filldir64);
+ up(&inode->i_sem);
if (error < 0)
- goto out;
+ goto out_f;
lastdirent = buf.previous;
if (!lastdirent) {
error = buf.error;
- goto out;
+ goto out_f;
}
lastdirent->d_off = (u64) file->f_pos;
#ifdef DEBUG_GETDENTS
@@ -2254,6 +2251,8 @@
#endif
error = cnt - buf.count;

+out_f:
+ fput(file);
out:
unlock_kernel();
return error;
\
 
 \ /
  Last update: 2005-03-22 13:51    [W:0.068 / U:0.144 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site