* [PATCH] 3 one-liner bugfixes
@ 2001-05-04 22:08 Manfred Spraul
2001-05-04 22:20 ` Linus Torvalds
0 siblings, 1 reply; 4+ messages in thread
From: Manfred Spraul @ 2001-05-04 22:08 UTC (permalink / raw)
To: torvalds; +Cc: linux-kernel
[-- Attachment #1: Type: text/plain, Size: 408 bytes --]
Hi Linus,
I found a 3 small bugs:
* mm/slab.c: the offslab_limit calculation used 2 instead of
sizeof(kmem_bufctl_t) [==4]. Cosmetic bug, since offslab_limit is never
reached.
* expand_stack is not down_read() safe, but used in the page-in path.
Fix is trivial.
* missing/wrong lock_kernel calls in fs/fcntl.c: getlk/setlk run without
the big kernel lock. The ..64 function acquire the lock.
--
Manfred
[-- Attachment #2: patch-slabbug-2 --]
[-- Type: text/plain, Size: 455 bytes --]
--- 2.4/mm/slab.c Sat Mar 3 17:58:05 2001
+++ build-2.4/mm/slab.c Sat Mar 3 19:57:16 2001
@@ -448,7 +448,7 @@
/* Inc off-slab bufctl limit until the ceiling is hit. */
if (!(OFF_SLAB(sizes->cs_cachep))) {
offslab_limit = sizes->cs_size-sizeof(slab_t);
- offslab_limit /= 2;
+ offslab_limit /= sizeof(kmem_bufctl_t);
}
sprintf(name, "size-%Zd(DMA)",sizes->cs_size);
sizes->cs_dmacachep = kmem_cache_create(name, sizes->cs_size, 0,
[-- Attachment #3: patch-expand-stack --]
[-- Type: text/plain, Size: 722 bytes --]
--- 2.4/include/linux/mm.h Mon Apr 30 23:14:10 2001
+++ build-2.4/include/linux/mm.h Fri May 4 23:14:35 2001
@@ -502,12 +502,14 @@
{
unsigned long grow;
+ spin_lock(&vma->vm_mm->page_table_lock);
address &= PAGE_MASK;
grow = (vma->vm_start - address) >> PAGE_SHIFT;
if (vma->vm_end - address > current->rlim[RLIMIT_STACK].rlim_cur ||
- ((vma->vm_mm->total_vm + grow) << PAGE_SHIFT) > current->rlim[RLIMIT_AS].rlim_cur)
+ ((vma->vm_mm->total_vm + grow) << PAGE_SHIFT) > current->rlim[RLIMIT_AS].rlim_cur) {
+ spin_unlock(&vma->vm_mm->page_table_lock);
return -ENOMEM;
- spin_lock(&vma->vm_mm->page_table_lock);
+ }
vma->vm_start = address;
vma->vm_pgoff -= grow;
vma->vm_mm->total_vm += grow;
[-- Attachment #4: patch-fcntl --]
[-- Type: text/plain, Size: 1014 bytes --]
--- 2.4/fs/fcntl.c Thu Nov 16 07:50:25 2000
+++ build-2.4/fs/fcntl.c Fri May 4 23:12:24 2001
@@ -254,11 +254,15 @@
unlock_kernel();
break;
case F_GETLK:
+ lock_kernel();
err = fcntl_getlk(fd, (struct flock *) arg);
+ unlock_kernel();
break;
case F_SETLK:
case F_SETLKW:
+ lock_kernel();
err = fcntl_setlk(fd, cmd, (struct flock *) arg);
+ unlock_kernel();
break;
case F_GETOWN:
/*
@@ -338,22 +342,26 @@
if (!filp)
goto out;
- lock_kernel();
switch (cmd) {
case F_GETLK64:
+ lock_kernel();
err = fcntl_getlk64(fd, (struct flock64 *) arg);
+ unlock_kernel();
break;
case F_SETLK64:
+ lock_kernel();
err = fcntl_setlk64(fd, cmd, (struct flock64 *) arg);
+ unlock_kernel();
break;
case F_SETLKW64:
+ lock_kernel();
err = fcntl_setlk64(fd, cmd, (struct flock64 *) arg);
+ unlock_kernel();
break;
default:
err = do_fcntl(fd, cmd, arg, filp);
break;
}
- unlock_kernel();
fput(filp);
out:
return err;
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] 3 one-liner bugfixes
2001-05-04 22:08 [PATCH] 3 one-liner bugfixes Manfred Spraul
@ 2001-05-04 22:20 ` Linus Torvalds
2001-05-04 23:25 ` Manfred Spraul
0 siblings, 1 reply; 4+ messages in thread
From: Linus Torvalds @ 2001-05-04 22:20 UTC (permalink / raw)
To: Manfred Spraul; +Cc: linux-kernel
On Sat, 5 May 2001, Manfred Spraul wrote:
>
> * missing/wrong lock_kernel calls in fs/fcntl.c: getlk/setlk run without
> the big kernel lock. The ..64 function acquire the lock.
This is wrong. The big lock (if it is needed, but I thought the current
locking should be safe) should be pushed down into the point where it is
needed, not at the caller..
Linus
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] 3 one-liner bugfixes
2001-05-04 22:20 ` Linus Torvalds
@ 2001-05-04 23:25 ` Manfred Spraul
2001-05-04 23:58 ` Manfred Spraul
0 siblings, 1 reply; 4+ messages in thread
From: Manfred Spraul @ 2001-05-04 23:25 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-kernel
[-- Attachment #1: Type: text/plain, Size: 821 bytes --]
Linus Torvalds wrote:
>
> On Sat, 5 May 2001, Manfred Spraul wrote:
> >
> > * missing/wrong lock_kernel calls in fs/fcntl.c: getlk/setlk run without
> > the big kernel lock. The ..64 function acquire the lock.
>
> This is wrong. The big lock (if it is needed, but I thought the current
> locking should be safe) should be pushed down into the point where it is
> needed, not at the caller..
>
Ok, I've removed the locks from fs/fcntl.c and added them into
fs/locks.c:
* fcntl_getlease dereferences filp->f_dentry->d_inode->i_flock. Race
with multithreaded app: sys_close()->filp_close()->locks_remove_posix()
+ fcntl_getlease()
* according to Documentation/filesystems/Locking, f_op->lock() is called
with the blk acquired. lock added around that call in
fcntl_{get,set}lk{,64}
I've attached a new patch.
--
Manfred
[-- Attachment #2: patch-fcntl --]
[-- Type: text/plain, Size: 1920 bytes --]
// $Header$
// Kernel Version:
// VERSION = 2
// PATCHLEVEL = 4
// SUBLEVEL = 4
// EXTRAVERSION =
--- 2.4/fs/fcntl.c Thu Nov 16 07:50:25 2000
+++ build-2.4/fs/fcntl.c Sat May 5 00:32:17 2001
@@ -338,7 +338,6 @@
if (!filp)
goto out;
- lock_kernel();
switch (cmd) {
case F_GETLK64:
err = fcntl_getlk64(fd, (struct flock64 *) arg);
@@ -353,7 +352,6 @@
err = do_fcntl(fd, cmd, arg, filp);
break;
}
- unlock_kernel();
fput(filp);
out:
return err;
--- 2.4/fs/locks.c Sun Apr 22 13:21:33 2001
+++ build-2.4/fs/locks.c Sat May 5 01:20:50 2001
@@ -1157,11 +1157,16 @@
int fcntl_getlease(struct file *filp)
{
struct file_lock *fl;
-
+ int ret;
+
+ lock_kernel();
fl = filp->f_dentry->d_inode->i_flock;
if ((fl == NULL) || ((fl->fl_flags & FL_LEASE) == 0))
- return F_UNLCK;
- return fl->fl_type & ~F_INPROGRESS;
+ ret = F_UNLCK;
+ else
+ fl->fl_type & ~F_INPROGRESS;
+ unlock_kernel();
+ return ret;
}
/* We already had a lease on this file; just change its type */
@@ -1357,7 +1362,9 @@
goto out_putf;
if (filp->f_op && filp->f_op->lock) {
+ lock_kernel();
error = filp->f_op->lock(filp, F_GETLK, &file_lock);
+ unlock_kernel();
if (error < 0)
goto out_putf;
else if (error == LOCK_USE_CLNT)
@@ -1481,7 +1488,9 @@
}
if (filp->f_op && filp->f_op->lock != NULL) {
+ lock_kernel();
error = filp->f_op->lock(filp, cmd, file_lock);
+ unlock_kernel();
if (error < 0)
goto out_putf;
}
@@ -1522,7 +1531,9 @@
goto out_putf;
if (filp->f_op && filp->f_op->lock) {
+ lock_kernel();
error = filp->f_op->lock(filp, F_GETLK, &file_lock);
+ unlock_kernel();
if (error < 0)
goto out_putf;
else if (error == LOCK_USE_CLNT)
@@ -1619,7 +1630,9 @@
}
if (filp->f_op && filp->f_op->lock != NULL) {
+ lock_kernel();
error = filp->f_op->lock(filp, cmd, file_lock);
+ unlock_kernel();
if (error < 0)
goto out_putf;
}
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] 3 one-liner bugfixes
2001-05-04 23:25 ` Manfred Spraul
@ 2001-05-04 23:58 ` Manfred Spraul
0 siblings, 0 replies; 4+ messages in thread
From: Manfred Spraul @ 2001-05-04 23:58 UTC (permalink / raw)
To: Linus Torvalds, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 234 bytes --]
Manfred Spraul wrote:
>
> + else
> + fl->fl_type & ~F_INPROGRESS;
^^^^^^
> + unlock_kernel();
> + return ret;
> }
The last patch was incorrect. Corrected version attached.
--
Manfred
[-- Attachment #2: patch-fcntl --]
[-- Type: text/plain, Size: 1925 bytes --]
// $Header$
// Kernel Version:
// VERSION = 2
// PATCHLEVEL = 4
// SUBLEVEL = 4
// EXTRAVERSION =
--- 2.4/fs/fcntl.c Thu Nov 16 07:50:25 2000
+++ build-2.4/fs/fcntl.c Sat May 5 00:32:17 2001
@@ -338,7 +338,6 @@
if (!filp)
goto out;
- lock_kernel();
switch (cmd) {
case F_GETLK64:
err = fcntl_getlk64(fd, (struct flock64 *) arg);
@@ -353,7 +352,6 @@
err = do_fcntl(fd, cmd, arg, filp);
break;
}
- unlock_kernel();
fput(filp);
out:
return err;
--- 2.4/fs/locks.c Sun Apr 22 13:21:33 2001
+++ build-2.4/fs/locks.c Sat May 5 01:54:59 2001
@@ -1157,11 +1157,16 @@
int fcntl_getlease(struct file *filp)
{
struct file_lock *fl;
-
+ int ret;
+
+ lock_kernel();
fl = filp->f_dentry->d_inode->i_flock;
if ((fl == NULL) || ((fl->fl_flags & FL_LEASE) == 0))
- return F_UNLCK;
- return fl->fl_type & ~F_INPROGRESS;
+ ret = F_UNLCK;
+ else
+ ret = fl->fl_type & ~F_INPROGRESS;
+ unlock_kernel();
+ return ret;
}
/* We already had a lease on this file; just change its type */
@@ -1357,7 +1362,9 @@
goto out_putf;
if (filp->f_op && filp->f_op->lock) {
+ lock_kernel();
error = filp->f_op->lock(filp, F_GETLK, &file_lock);
+ unlock_kernel();
if (error < 0)
goto out_putf;
else if (error == LOCK_USE_CLNT)
@@ -1481,7 +1488,9 @@
}
if (filp->f_op && filp->f_op->lock != NULL) {
+ lock_kernel();
error = filp->f_op->lock(filp, cmd, file_lock);
+ unlock_kernel();
if (error < 0)
goto out_putf;
}
@@ -1522,7 +1531,9 @@
goto out_putf;
if (filp->f_op && filp->f_op->lock) {
+ lock_kernel();
error = filp->f_op->lock(filp, F_GETLK, &file_lock);
+ unlock_kernel();
if (error < 0)
goto out_putf;
else if (error == LOCK_USE_CLNT)
@@ -1619,7 +1630,9 @@
}
if (filp->f_op && filp->f_op->lock != NULL) {
+ lock_kernel();
error = filp->f_op->lock(filp, cmd, file_lock);
+ unlock_kernel();
if (error < 0)
goto out_putf;
}
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2001-05-04 23:59 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2001-05-04 22:08 [PATCH] 3 one-liner bugfixes Manfred Spraul
2001-05-04 22:20 ` Linus Torvalds
2001-05-04 23:25 ` Manfred Spraul
2001-05-04 23:58 ` Manfred Spraul
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox