Commit 5ffe57da authored by Cong Wang's avatar Cong Wang Committed by David S. Miller
Browse files

act_ife: fix a potential deadlock 



use_all_metadata() acquires read_lock(&ife_mod_lock), then calls
add_metainfo() which calls find_ife_oplist() which acquires the same
lock again. Deadlock!

Introduce __add_metainfo() which accepts struct tcf_meta_ops *ops
as an additional parameter and let its callers to decide how
to find it. For use_all_metadata(), it already has ops, no
need to find it again, just call __add_metainfo() directly.

And, as ife_mod_lock is only needed for find_ife_oplist(),
this means we can make non-atomic allocation for populate_metalist()
now.

Fixes: 817e9f2c ("act_ife: acquire ife_mod_lock before reading ifeoplist")
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: default avatarCong Wang <xiyou.wangcong@gmail.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 4e407ff5

net/sched/act_ife.c

+21 −13
Original line number Diff line number Diff line
@@ -296,22 +296,16 @@ static int load_metaops_and_vet(u32 metaid, void *val, int len, bool rtnl_held) 


/* called when adding new meta information

*/

static int add_metainfo(struct tcf_ife_info *ife, u32 metaid, void *metaval,

static int __add_metainfo(const struct tcf_meta_ops *ops,

			  struct tcf_ife_info *ife, u32 metaid, void *metaval,

			  int len, bool atomic, bool exists)

{

	struct tcf_meta_info *mi = NULL;

	struct tcf_meta_ops *ops = find_ife_oplist(metaid);

	int ret = 0;



	if (!ops)

		return -ENOENT;



	mi = kzalloc(sizeof(*mi), atomic ? GFP_ATOMIC : GFP_KERNEL);

	if (!mi) {

		/*put back what find_ife_oplist took */

		module_put(ops->owner);

	if (!mi)

		return -ENOMEM;

	}



	mi->metaid = metaid;

	mi->ops = ops;
@@ -319,7 +313,6 @@ static int add_metainfo(struct tcf_ife_info *ife, u32 metaid, void *metaval, 
		ret = ops->alloc(mi, metaval, atomic ? GFP_ATOMIC : GFP_KERNEL);

		if (ret != 0) {

			kfree(mi);

			module_put(ops->owner);

			return ret;

		}

	}
@@ -333,6 +326,21 @@ static int add_metainfo(struct tcf_ife_info *ife, u32 metaid, void *metaval, 
	return ret;

}



static int add_metainfo(struct tcf_ife_info *ife, u32 metaid, void *metaval,

			int len, bool exists)

{

	const struct tcf_meta_ops *ops = find_ife_oplist(metaid);

	int ret;



	if (!ops)

		return -ENOENT;

	ret = __add_metainfo(ops, ife, metaid, metaval, len, false, exists);

	if (ret)

		/*put back what find_ife_oplist took */

		module_put(ops->owner);

	return ret;

}



static int use_all_metadata(struct tcf_ife_info *ife, bool exists)

{

	struct tcf_meta_ops *o;
@@ -341,7 +349,7 @@ static int use_all_metadata(struct tcf_ife_info *ife, bool exists) 


	read_lock(&ife_mod_lock);

	list_for_each_entry(o, &ifeoplist, list) {

		rc = add_metainfo(ife, o->metaid, NULL, 0, true, exists);

		rc = __add_metainfo(o, ife, o->metaid, NULL, 0, true, exists);

		if (rc == 0)

			installed += 1;

	}
@@ -435,7 +443,7 @@ static int populate_metalist(struct tcf_ife_info *ife, struct nlattr **tb, 
			if (rc != 0)

				return rc;



			rc = add_metainfo(ife, i, val, len, false, exists);

			rc = add_metainfo(ife, i, val, len, exists);

			if (rc)

				return rc;

		}

CRA Git | Maintained and supported by SUSTech CRA and CCSE