netdev - Re: [RFC RTNETLINK 04/09]: Link creation API

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]

Message-ID: <m1hcplt102.fsf@ebiederm.dsl.xmission.com>
Date:	Wed, 06 Jun 2007 12:02:21 -0600
From:	ebiederm@...ssion.com (Eric W. Biederman)
To:	Patrick McHardy <kaber@...sh.net>
Cc:	netdev@...r.kernel.org, socketcan@...tkopp.net, hadi@...erus.ca,
	xemul@...ru, tgraf@...g.ch
Subject: Re: [RFC RTNETLINK 04/09]: Link creation API

Patrick McHardy <kaber@...sh.net> writes:

> Eric W. Biederman wrote:
>>>+	if (linkinfo[IFLA_INFO_NAME]) {
>>>+		nla_strlcpy(name, linkinfo[IFLA_INFO_NAME], sizeof(name));
>>>+		ops = rtnl_link_ops_get(name);
>> 
>> 
>> Ugh.  Shouldn't we have the request_module logic here?
>> Otherwise it looks like we can skip the validate method and 
>> have other weird interactions.
>
>
> Good catch. The easiest solution seems be to simply replay the
> request after successful module load, which also avoids the
> device lookup race.

Looks reasonable to me.   There might be a variable or two that
we need to make certain is initialized but at a quick glance it
looks ok.

Eric


> Something like this (untested).
>
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index 8d2f817..f2868b0 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -930,6 +930,7 @@ static int rtnl_newlink(struct sk_buff *skb, struct nlmsghdr
> *nlh, void *arg)
>  	struct nlattr *linkinfo[IFLA_INFO_MAX+1];
>  	int err;
>  
> +replay:
>  	err = nlmsg_parse(nlh, sizeof(*ifm), tb, IFLA_MAX, ifla_policy);
>  	if (err < 0)
>  		return err;
> @@ -1012,19 +1013,19 @@ static int rtnl_newlink(struct sk_buff *skb, struct
> nlmsghdr *nlh, void *arg)
>  		if (tb[IFLA_ADDRESS] || tb[IFLA_BROADCAST] || tb[IFLA_MAP])
>  			return -EOPNOTSUPP;
>  
> +		if (!ops) {
>  #ifdef CONFIG_KMOD
> -		if (!ops && kind[0]) {
> -			/* race condition: device may be created while rtnl is
> -			 * unlocked, final register_netdevice will catch it.
> -			 */
> -			__rtnl_unlock();
> -			request_module("rtnl-link-%s", kind);
> -			rtnl_lock();
> -			ops = rtnl_link_ops_get(kind);
> -		}
> +			if (kind[0]) {
> +				__rtnl_unlock();
> +				request_module("rtnl-link-%s", kind);
> +				rtnl_lock();
> +				ops = rtnl_link_ops_get(kind);
> +				if (ops)
> +					goto replay;
> +			}
>  #endif
> -		if (!ops)
>  			return -EOPNOTSUPP;
> +		}
>  
>  		if (!ifname[0])
>  			snprintf(ifname, IFNAMSIZ, "%s%%d", ops->kind);
-
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