Re: [PATCH] hfsplus: release bnode pages after use, not before

[Vyacheslav Dubeyko <slava@dubeyko.com>]

On Sun, 2015-06-07 at 02:42 +0200, Sergei Antonov wrote:
> Fix this bugreport by Sasha Levin:
> http://lkml.org/lkml/2015/2/20/85 ("use after free")
> Make sure mapped pages are available for the entire lifetime of hfs_bnode.
> 

Sorry, I missed the point. What do you try to fix? How this change fixes
the issue?

I think that maybe this fix makes sense. But it needs to describe it
more deeply. Could you describe the fix with more details?

Thanks,
Vyacheslav Dubeyko.


> Cc: Anton Altaparmakov <aia21@cam.ac.uk>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> Cc: Christoph Hellwig <hch@infradead.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Vyacheslav Dubeyko <slava@dubeyko.com>
> Cc: Hin-Tak Leung <htl10@users.sourceforge.net>
> Cc: Sougata Santra <sougata@tuxera.com>
> Reported-by: Sasha Levin <sasha.levin@oracle.com>
> Signed-off-by: Sergei Antonov <saproj@gmail.com>
> ---
>  fs/hfsplus/bnode.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/hfsplus/bnode.c b/fs/hfsplus/bnode.c
> index 759708f..5af50fb 100644
> --- a/fs/hfsplus/bnode.c
> +++ b/fs/hfsplus/bnode.c
> @@ -454,7 +454,6 @@ static struct hfs_bnode *__hfs_bnode_create(struct hfs_btree *tree, u32 cnid)
>  			page_cache_release(page);
>  			goto fail;
>  		}
> -		page_cache_release(page);
>  		node->page[i] = page;
>  	}
>  
> @@ -566,13 +565,12 @@ node_error:
>  
>  void hfs_bnode_free(struct hfs_bnode *node)
>  {
> -#if 0
>  	int i;
>  
> -	for (i = 0; i < node->tree->pages_per_bnode; i++)
> +	for (i = 0; i < node->tree->pages_per_bnode; i++) {
>  		if (node->page[i])
>  			page_cache_release(node->page[i]);
> -#endif
> +	}
>  	kfree(node);
>  }
>