Re: [PATCH 2/4] nfsd: Can leak pnfs_block_extent on error

["J. Bruce Fields" <bfields@fieldses.org>]

On Wed, May 25, 2016 at 06:12:25PM +0000, Thomas Haynes wrote:
> 
> > On May 25, 2016, at 8:07 AM, Christoph Hellwig <hch@lst.de> wrote:
> > 
> > On Tue, May 24, 2016 at 10:09:37PM -0700, Tom Haynes wrote:
> >> Signed-off-by: Tom Haynes <loghyr@primarydata.com>
> > 
> > How was this reported?
> 
> Code inspection. My guess is no one ever hit the error cases
> in there.
> 
> > 
> > Like other NFS procedures the private data should be freed by the
> > XDR encode callback (nfsd4_encode_layoutget in this case) even
> > in the error case.  It could be that there is a bug somewhere,
> > but it probably shouldn't be fixed here.
> > 
> 
> No, it doesn’t do that on errors:

We have in nfsd4_block_proc_layoutget:

	bex = kzalloc(sizeof(*bex), GFP_KERNEL);
	if (!bex)
		goto out_error;
	args->lg_content = bex;

and then in nfsd4_encode_layoutget:

	kfree(lgp->lg_content);

So, I think we're OK as is?

--b.


> 
> nfsd4_layoutget():
> 
>        nfserr = ops->proc_layoutget(d_inode(current_fh->fh_dentry),
>                                      current_fh, lgp);
>         if (nfserr)
>                 goto out_put_stid;
> 
>         nfserr = nfsd4_insert_layout(lgp, ls);
> 
> out_put_stid:
>         mutex_unlock(&ls->ls_mutex);
>         nfs4_put_stid(&ls->ls_stid);
> out:
>         return nfserr;
> }
> 
> So on error we never do anything with the lgp and the memory would
> be dropped.