Commit b5fda08e authored by Zhihao Cheng's avatar Zhihao Cheng Committed by Richard Weinberger

ubifs: Fix memleak when insert_old_idx() failed

Following process will cause a memleak for copied up znode:

dirty_cow_znode
  zn = copy_znode(c, znode);
  err = insert_old_idx(c, zbr->lnum, zbr->offs);
  if (unlikely(err))
     return ERR_PTR(err);   // No one refers to zn.

Fetch a reproducer in [Link].

Function copy_znode() is split into 2 parts: resource allocation
and znode replacement, insert_old_idx() is split in similar way,
so resource cleanup could be done in error handling path without
corrupting metadata(mem & disk).
It's okay that old index inserting is put behind of add_idx_dirt(),
old index is used in layout_leb_in_gaps(), so the two processes do
not depend on each other.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=216705
Fixes: 1e51764a ("UBIFS: add new flash file system")
Cc: stable@vger.kernel.org
Signed-off-by: default avatarZhihao Cheng <chengzhihao1@huawei.com>
Signed-off-by: default avatarRichard Weinberger <richard@nod.at>
parent 7d01cb27
...@@ -44,6 +44,33 @@ enum { ...@@ -44,6 +44,33 @@ enum {
NOT_ON_MEDIA = 3, NOT_ON_MEDIA = 3,
}; };
static void do_insert_old_idx(struct ubifs_info *c,
struct ubifs_old_idx *old_idx)
{
struct ubifs_old_idx *o;
struct rb_node **p, *parent = NULL;
p = &c->old_idx.rb_node;
while (*p) {
parent = *p;
o = rb_entry(parent, struct ubifs_old_idx, rb);
if (old_idx->lnum < o->lnum)
p = &(*p)->rb_left;
else if (old_idx->lnum > o->lnum)
p = &(*p)->rb_right;
else if (old_idx->offs < o->offs)
p = &(*p)->rb_left;
else if (old_idx->offs > o->offs)
p = &(*p)->rb_right;
else {
ubifs_err(c, "old idx added twice!");
kfree(old_idx);
}
}
rb_link_node(&old_idx->rb, parent, p);
rb_insert_color(&old_idx->rb, &c->old_idx);
}
/** /**
* insert_old_idx - record an index node obsoleted since the last commit start. * insert_old_idx - record an index node obsoleted since the last commit start.
* @c: UBIFS file-system description object * @c: UBIFS file-system description object
...@@ -69,35 +96,15 @@ enum { ...@@ -69,35 +96,15 @@ enum {
*/ */
static int insert_old_idx(struct ubifs_info *c, int lnum, int offs) static int insert_old_idx(struct ubifs_info *c, int lnum, int offs)
{ {
struct ubifs_old_idx *old_idx, *o; struct ubifs_old_idx *old_idx;
struct rb_node **p, *parent = NULL;
old_idx = kmalloc(sizeof(struct ubifs_old_idx), GFP_NOFS); old_idx = kmalloc(sizeof(struct ubifs_old_idx), GFP_NOFS);
if (unlikely(!old_idx)) if (unlikely(!old_idx))
return -ENOMEM; return -ENOMEM;
old_idx->lnum = lnum; old_idx->lnum = lnum;
old_idx->offs = offs; old_idx->offs = offs;
do_insert_old_idx(c, old_idx);
p = &c->old_idx.rb_node;
while (*p) {
parent = *p;
o = rb_entry(parent, struct ubifs_old_idx, rb);
if (lnum < o->lnum)
p = &(*p)->rb_left;
else if (lnum > o->lnum)
p = &(*p)->rb_right;
else if (offs < o->offs)
p = &(*p)->rb_left;
else if (offs > o->offs)
p = &(*p)->rb_right;
else {
ubifs_err(c, "old idx added twice!");
kfree(old_idx);
return 0;
}
}
rb_link_node(&old_idx->rb, parent, p);
rb_insert_color(&old_idx->rb, &c->old_idx);
return 0; return 0;
} }
...@@ -199,23 +206,6 @@ static struct ubifs_znode *copy_znode(struct ubifs_info *c, ...@@ -199,23 +206,6 @@ static struct ubifs_znode *copy_znode(struct ubifs_info *c,
__set_bit(DIRTY_ZNODE, &zn->flags); __set_bit(DIRTY_ZNODE, &zn->flags);
__clear_bit(COW_ZNODE, &zn->flags); __clear_bit(COW_ZNODE, &zn->flags);
ubifs_assert(c, !ubifs_zn_obsolete(znode));
__set_bit(OBSOLETE_ZNODE, &znode->flags);
if (znode->level != 0) {
int i;
const int n = zn->child_cnt;
/* The children now have new parent */
for (i = 0; i < n; i++) {
struct ubifs_zbranch *zbr = &zn->zbranch[i];
if (zbr->znode)
zbr->znode->parent = zn;
}
}
atomic_long_inc(&c->dirty_zn_cnt);
return zn; return zn;
} }
...@@ -233,6 +223,42 @@ static int add_idx_dirt(struct ubifs_info *c, int lnum, int dirt) ...@@ -233,6 +223,42 @@ static int add_idx_dirt(struct ubifs_info *c, int lnum, int dirt)
return ubifs_add_dirt(c, lnum, dirt); return ubifs_add_dirt(c, lnum, dirt);
} }
/**
* replace_znode - replace old znode with new znode.
* @c: UBIFS file-system description object
* @new_zn: new znode
* @old_zn: old znode
* @zbr: the branch of parent znode
*
* Replace old znode with new znode in TNC.
*/
static void replace_znode(struct ubifs_info *c, struct ubifs_znode *new_zn,
struct ubifs_znode *old_zn, struct ubifs_zbranch *zbr)
{
ubifs_assert(c, !ubifs_zn_obsolete(old_zn));
__set_bit(OBSOLETE_ZNODE, &old_zn->flags);
if (old_zn->level != 0) {
int i;
const int n = new_zn->child_cnt;
/* The children now have new parent */
for (i = 0; i < n; i++) {
struct ubifs_zbranch *child = &new_zn->zbranch[i];
if (child->znode)
child->znode->parent = new_zn;
}
}
zbr->znode = new_zn;
zbr->lnum = 0;
zbr->offs = 0;
zbr->len = 0;
atomic_long_inc(&c->dirty_zn_cnt);
}
/** /**
* dirty_cow_znode - ensure a znode is not being committed. * dirty_cow_znode - ensure a znode is not being committed.
* @c: UBIFS file-system description object * @c: UBIFS file-system description object
...@@ -265,21 +291,32 @@ static struct ubifs_znode *dirty_cow_znode(struct ubifs_info *c, ...@@ -265,21 +291,32 @@ static struct ubifs_znode *dirty_cow_znode(struct ubifs_info *c,
return zn; return zn;
if (zbr->len) { if (zbr->len) {
err = insert_old_idx(c, zbr->lnum, zbr->offs); struct ubifs_old_idx *old_idx;
if (unlikely(err))
return ERR_PTR(err); old_idx = kmalloc(sizeof(struct ubifs_old_idx), GFP_NOFS);
if (unlikely(!old_idx)) {
err = -ENOMEM;
goto out;
}
old_idx->lnum = zbr->lnum;
old_idx->offs = zbr->offs;
err = add_idx_dirt(c, zbr->lnum, zbr->len); err = add_idx_dirt(c, zbr->lnum, zbr->len);
} else if (err) {
err = 0; kfree(old_idx);
goto out;
}
zbr->znode = zn; do_insert_old_idx(c, old_idx);
zbr->lnum = 0; }
zbr->offs = 0;
zbr->len = 0; replace_znode(c, zn, znode, zbr);
if (unlikely(err))
return ERR_PTR(err);
return zn; return zn;
out:
kfree(zn);
return ERR_PTR(err);
} }
/** /**
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment