linux-kernel - Re: [PATCHv3] locks: prevent side-effects of locks_release_private before file

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20120729155642.GJ6388@fieldses.org>
Date:	Sun, 29 Jul 2012 11:56:42 -0400
From:	"J. Bruce Fields" <bfields@...ldses.org>
To:	Filipe Brandenburger <filbranden@...il.com>
Cc:	Al Viro <viro@...iv.linux.org.uk>, Matthew Wilcox <matthew@....cx>,
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCHv3] locks: prevent side-effects of locks_release_private
 before file_lock is initialized

On Fri, Jul 27, 2012 at 04:45:52PM -0400, J. Bruce Fields wrote:
> 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).

(Committing this as well.)--b.

> 
> --b.
> 
> commit 96d6d59ceaeaacba4088862f3c57fcd011f52832
> Author: J. Bruce Fields <bfields@...hat.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@...hat.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;
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/