Messages in this thread Patch in this message | | | Date | Fri, 27 Jul 2012 16:45:52 -0400 | From | "J. Bruce Fields" <> | Subject | Re: [PATCHv3] locks: prevent side-effects of locks_release_private before file_lock is initialized |
| |
On Fri, Jul 27, 2012 at 12:42:52AM -0400, Filipe Brandenburger wrote: > When calling fcntl(fd, F_SETLEASE, lck) [with lck=F_WRLCK or F_RDLCK], > the custom signal or owner (if any were previously set using F_SETSIG > or F_SETOWN fcntls) would be reset when F_SETLEASE was called for the > second time on the same file descriptor. > > This bug is a regression of 2.6.37 and is described here: > https://bugzilla.kernel.org/show_bug.cgi?id=43336 > > This patch reverts a commit from Oct 2004 (with subject "nfs4 lease: > move the f_delown processing") which originally introduced the > lm_release_private callback.
Looks fine, thanks. I think can also do something like the following (on top of your patch).
--b.
commit 96d6d59ceaeaacba4088862f3c57fcd011f52832 Author: J. Bruce Fields <bfields@redhat.com> Date: Fri Jul 27 16:18:00 2012 -0400
locks: move lease-specific code out of locks_delete_lock
No point putting something only used by one caller into common code.
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
diff --git a/fs/locks.c b/fs/locks.c index 86668dd..541075a 100644 --- a/fs/locks.c +++ b/fs/locks.c @@ -570,12 +570,6 @@ static void locks_delete_lock(struct file_lock **thisfl_p) fl->fl_next = NULL; list_del_init(&fl->fl_link); - fasync_helper(0, fl->fl_file, 0, &fl->fl_fasync); - if (fl->fl_fasync != NULL) { - printk(KERN_ERR "locks_delete_lock: fasync == %p\n", fl->fl_fasync); - fl->fl_fasync = NULL; - } - if (fl->fl_nspid) { put_pid(fl->fl_nspid); fl->fl_nspid = NULL; @@ -1150,6 +1144,11 @@ int lease_modify(struct file_lock **before, int arg) f_delown(filp); filp->f_owner.signum = 0; + fasync_helper(0, fl->fl_file, 0, &fl->fl_fasync); + if (fl->fl_fasync != NULL) { + printk(KERN_ERR "locks_delete_lock: fasync == %p\n", fl->fl_fasync); + fl->fl_fasync = NULL; + } locks_delete_lock(before); } return 0;
| |