| Message ID | 1543025458-31592-1-git-send-email-bianpan2016@163.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [V2] hfs: do not free node before using | expand |
On Sat, 2018-11-24 at 10:10 +0800, Pan Bian wrote: > The function hfs_bmap_free frees node via hfs_bnode_put(node). > However, > it then reads node->this when dumping error message on an error path, > which may result in a use-after-free bug. This patch frees node only > when it is never used. > > Fixes: a1185ffa2fc("HFS rewrite") > > Signed-off-by: Pan Bian <bianpan2016@163.com> > > --- > V2: correct the commit information in Fixes By the way, HFS+ has the same issue [1]. > > --- > fs/hfs/btree.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/fs/hfs/btree.c b/fs/hfs/btree.c > index 98b96ff..19017d2 100644 > --- a/fs/hfs/btree.c > +++ b/fs/hfs/btree.c > @@ -338,13 +338,14 @@ void hfs_bmap_free(struct hfs_bnode *node) > > nidx -= len * 8; > i = node->next; > - hfs_bnode_put(node); > if (!i) { > /* panic */; > pr_crit("unable to free bnode %u. bmap not > found!\n", > node->this); > + hfs_bnode_put(node); > return; > } > + hfs_bnode_put(node); First of all, it looks weird to have two calls of hfs_bnode_put(node). Secondly, you will add only one line of code instead of two if you simply save the node->this in the local variable. > node = hfs_bnode_find(tree, i); > if (IS_ERR(node)) > return; Thanks, Vyacheslav Dubeyko. [1] https://elixir.bootlin.com/linux/latest/source/fs/hfsplus/btree.c#L457
diff --git a/fs/hfs/btree.c b/fs/hfs/btree.c index 98b96ff..19017d2 100644 --- a/fs/hfs/btree.c +++ b/fs/hfs/btree.c @@ -338,13 +338,14 @@ void hfs_bmap_free(struct hfs_bnode *node) nidx -= len * 8; i = node->next; - hfs_bnode_put(node); if (!i) { /* panic */; pr_crit("unable to free bnode %u. bmap not found!\n", node->this); + hfs_bnode_put(node); return; } + hfs_bnode_put(node); node = hfs_bnode_find(tree, i); if (IS_ERR(node)) return;
The function hfs_bmap_free frees node via hfs_bnode_put(node). However, it then reads node->this when dumping error message on an error path, which may result in a use-after-free bug. This patch frees node only when it is never used. Fixes: a1185ffa2fc("HFS rewrite") Signed-off-by: Pan Bian <bianpan2016@163.com> --- V2: correct the commit information in Fixes --- fs/hfs/btree.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)