Re: [ 029/100] NFSv4.1: Hold reference to layout hdr in layoutget

[Ben Hutchings]

On Tue, 2013-03-12 at 15:31 -0700, Greg Kroah-Hartman wrote:
> 3.8-stable review patch.  If anyone has any objections, please let me know.
> 
> ------------------
> 
> From: Weston Andros Adamson <dros@xxxxxxxxxx>
> 
> commit a47970ff7814718fec31b7d966747c6aa1a3545f upstream.
[....]
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -6087,11 +6087,13 @@ static struct page **nfs4_alloc_pages(si
>  static void nfs4_layoutget_release(void *calldata)
>  {
>  	struct nfs4_layoutget *lgp = calldata;
> -	struct nfs_server *server = NFS_SERVER(lgp->args.inode);
> +	struct inode *inode = lgp->args.inode;
> +	struct nfs_server *server = NFS_SERVER(inode);
>  	size_t max_pages = max_response_pages(server);
>  
>  	dprintk("--> %s\n", __func__);
>  	nfs4_free_pages(lgp->args.layout.pages, max_pages);
> +	pnfs_put_layout_hdr(NFS_I(inode)->layout);
>  	put_nfs_open_context(lgp->args.ctx);
>  	kfree(calldata);
>  	dprintk("<-- %s\n", __func__);
> @@ -6106,7 +6108,8 @@ static const struct rpc_call_ops nfs4_la
>  struct pnfs_layout_segment *
>  nfs4_proc_layoutget(struct nfs4_layoutget *lgp, gfp_t gfp_flags)
>  {
> -	struct nfs_server *server = NFS_SERVER(lgp->args.inode);
> +	struct inode *inode = lgp->args.inode;
> +	struct nfs_server *server = NFS_SERVER(inode);
>  	size_t max_pages = max_response_pages(server);
>  	struct rpc_task *task;
>  	struct rpc_message msg = {
> @@ -6136,6 +6139,10 @@ nfs4_proc_layoutget(struct nfs4_layoutge
>  	lgp->res.layoutp = &lgp->args.layout;
>  	lgp->res.seq_res.sr_slot = NULL;
>  	nfs41_init_sequence(&lgp->args.seq_args, &lgp->res.seq_res, 0);
> +
> +	/* nfs4_layoutget_release calls pnfs_put_layout_hdr */
> +	pnfs_get_layout_hdr(NFS_I(inode)->layout);
> +

But this function also calls nfs4_layoutget_release() if
nfs4_alloc_pages() fails, i.e. before it calls pnfs_get_layout_hdr().
This will lead to a reference imbalance.

Ben.

>  	task = rpc_run_task(&task_setup_data);
>  	if (IS_ERR(task))
>  		return ERR_CAST(task);
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe stable" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-- 
Ben Hutchings
Humans are not rational beings; they are rationalising beings.

Attachment: signature.asc
Description: This is a digitally signed message part