netdev - Re: [PATCH 2/2] genl: Hold reference on correct module while netlink-dump.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 22 Aug 2013 09:40:42 +0200
From:	Johannes Berg <johannes@...solutions.net>
To:	Pravin B Shelar <pshelar@...ira.com>
Cc:	netdev@...r.kernel.org, Jesse Gross <jesse@...ira.com>
Subject: Re: [PATCH 2/2] genl: Hold reference on correct module while
 netlink-dump.

On Wed, 2013-08-21 at 20:58 -0700, Pravin B Shelar wrote:
> netlink dump operations take module as parameter to hold
> reference for entire netlink dump duration.
> Currently it holds ref only on genl module which is not correct
> when we use ops registered to genl from another module.
> Following patch adds module pointer to genl_ops so that netlink
> can hold ref count on it.
> 
> CC: Jesse Gross <jesse@...ira.com>
> CC: Johannes Berg <johannes.berg@...el.com>
> Signed-off-by: Pravin B Shelar <pshelar@...ira.com>
> ---
>  include/net/genetlink.h |   21 ++++++++++++++++++---
>  net/netlink/genetlink.c |   39 ++++++++++++++++++++++++---------------
>  2 files changed, 42 insertions(+), 18 deletions(-)
> 
> diff --git a/include/net/genetlink.h b/include/net/genetlink.h
> index 93024a4..7f57b2c 100644
> --- a/include/net/genetlink.h
> +++ b/include/net/genetlink.h
> @@ -119,13 +119,28 @@ struct genl_ops {
>  					 struct netlink_callback *cb);
>  	int		       (*done)(struct netlink_callback *cb);
>  	struct list_head	ops_list;
> +	struct module		*module;
>  };

This may be more correct than my patch, but I'm not sure it's worth
spending the memory. Is there going to be any generic netlink family
that actually puts operations into a different module than the family
itself? I doubt that.

> @@ -605,14 +610,18 @@ static int genl_family_rcv_msg(struct genl_family *family,
>  			genl_unlock();
>  			c.data = ops;
>  			c.dump = genl_lock_dumpit;
> -			if (ops->done)
> -				c.done = genl_lock_done;
> +			c.done = genl_lock_done;
> +			c.module = THIS_MODULE;

THIS_MODULE here is useless, this code is always built-in.

> +			if (!try_module_get(ops->module))
> +				return -EPROTONOSUPPORT;

Why open-code it? You can just point c.module to the ops module here as
well (because generic netlink is built-in) and save yourself the
try_module_get stuff.

johannes

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html