Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 6f2eee54 authored by Zhihao Cheng's avatar Zhihao Cheng Committed by Greg Kroah-Hartman
Browse files

ubifs: Fix memleak when insert_old_idx() failed 

commit b5fda08ef213352ac2df7447611eb4d383cce929 upstream.

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>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 2c2a76b7

fs/ubifs/tnc.c

+87 −50
Original line number Original line Diff line number Diff line
@@ -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 { 
 */

 */

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, 
	__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) 
	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, 
		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);

}

}





/**

/**