linux-kernel - Re: [PATCH 2/2] sysfs: only access bin file vm

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.LSU.2.00.1009221231251.29127@tigran.mtv.corp.google.com>
Date:	Wed, 22 Sep 2010 12:49:51 -0700 (PDT)
From:	Hugh Dickins <hughd@...gle.com>
To:	"Eric W. Biederman" <ebiederm@...ssion.com>
cc:	Greg Kroah-Hartman <gregkh@...e.de>, linux-kernel@...r.kernel.org,
	Tejun Heo <tj@...nel.org>
Subject: Re: [PATCH 2/2] sysfs: only access bin file vm_ops with the active
 lock

On Mon, 20 Sep 2010, Eric W. Biederman wrote:
> 
> bb->vm_ops is a cached copy of the vm_ops of the underlying
> sysfs bin file, which means that after sysfs_bin_remove_file
> completes it is only longer valid to deference bb->vm_ops.
> 
> So move all of the tests of bb->vm_ops inside of where
> we hold the sysfs active lock.
> 
> Signed-off-by: Eric W. Biederman <ebiederm@...stanetworks.com>

Good point.  There's a couple of cases (the ones below) where you've
changed the return value from success to failure when the entry point
didn't exist for an object that is no longer active; but on reflection,
I accept that's just not worth worrying about.

Hugh

> @@ -219,13 +222,12 @@ static int bin_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
>  	if (!bb->vm_ops)
>  		return VM_FAULT_SIGBUS;
>  
> -	if (!bb->vm_ops->page_mkwrite)
> -		return 0;
> -
>  	if (!sysfs_get_active(attr_sd))
>  		return VM_FAULT_SIGBUS;
>  
> -	ret = bb->vm_ops->page_mkwrite(vma, vmf);
> +	ret = 0;
> +	if (bb->vm_ops->page_mkwrite)
> +		ret = bb->vm_ops->page_mkwrite(vma, vmf);
>  
>  	sysfs_put_active(attr_sd);
>  	return ret;

> @@ -259,13 +263,15 @@ static int bin_set_policy(struct vm_area_struct *vma, struct mempolicy *new)
>  	struct sysfs_dirent *attr_sd = file->f_path.dentry->d_fsdata;
>  	int ret;
>  
> -	if (!bb->vm_ops || !bb->vm_ops->set_policy)
> +	if (!bb->vm_ops)
>  		return 0;
>  
>  	if (!sysfs_get_active(attr_sd))
>  		return -EINVAL;
>  
> -	ret = bb->vm_ops->set_policy(vma, new);
> +	ret = 0;
> +	if (bb->vm_ops->set_policy)
> +		ret = bb->vm_ops->set_policy(vma, new);
>  
>  	sysfs_put_active(attr_sd);
>  	return ret;
--
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/