netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next 0/3] lwtunnel: autoload of lwt modules
@ 2016-02-15 15:42 Robert Shearman
  2016-02-15 15:42 ` [PATCH net-next 1/3] " Robert Shearman
                   ` (3 more replies)
  0 siblings, 4 replies; 15+ messages in thread
From: Robert Shearman @ 2016-02-15 15:42 UTC (permalink / raw)
  To: davem
  Cc: netdev, Roopa Prabhu, Tom Herbert, Thomas Graf,
	Eric W. Biederman, Jiri Benc, Robert Shearman

The lwt implementations using net devices can autoload using the
existing mechanism using IFLA_INFO_KIND. However, there's no mechanism
that lwt modules not using net devices can use.

Therefore, add the ability to autoload modules registering lwt
operations for lwt implementations not using a net device so that
users don't have to manually load the modules.

Robert Shearman (3):
  lwtunnel: autoload of lwt modules
  mpls: autoload lwt module
  ila: autoload module

 include/net/lwtunnel.h    |  4 +++-
 net/core/lwtunnel.c       | 32 ++++++++++++++++++++++++++++++++
 net/ipv6/ila/ila_common.c |  1 +
 net/mpls/mpls_iptunnel.c  |  1 +
 4 files changed, 37 insertions(+), 1 deletion(-)

-- 
2.1.4

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH net-next 1/3] lwtunnel: autoload of lwt modules
  2016-02-15 15:42 [PATCH net-next 0/3] lwtunnel: autoload of lwt modules Robert Shearman
@ 2016-02-15 15:42 ` Robert Shearman
  2016-02-15 16:02   ` Jiri Benc
  2016-02-15 21:33   ` Eric W. Biederman
  2016-02-15 15:42 ` [PATCH net-next 2/3] mpls: autoload lwt module Robert Shearman
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 15+ messages in thread
From: Robert Shearman @ 2016-02-15 15:42 UTC (permalink / raw)
  To: davem
  Cc: netdev, Roopa Prabhu, Tom Herbert, Thomas Graf,
	Eric W. Biederman, Jiri Benc, Robert Shearman

The lwt implementations using net devices can autoload using the
existing mechanism using IFLA_INFO_KIND. However, there's no mechanism
that lwt modules not using net devices can use.

Therefore, add the ability to autoload modules registering lwt
operations for lwt implementations not using a net device so that
users don't have to manually load the modules.

Signed-off-by: Robert Shearman <rshearma@brocade.com>
---
 include/net/lwtunnel.h |  4 +++-
 net/core/lwtunnel.c    | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/include/net/lwtunnel.h b/include/net/lwtunnel.h
index 66350ce3e955..e9f116e29c22 100644
--- a/include/net/lwtunnel.h
+++ b/include/net/lwtunnel.h
@@ -170,6 +170,8 @@ static inline int lwtunnel_input(struct sk_buff *skb)
 	return -EOPNOTSUPP;
 }
 
-#endif
+#endif /* CONFIG_LWTUNNEL */
+
+#define MODULE_ALIAS_RTNL_LWT(encap_type) MODULE_ALIAS("rtnl-lwt-" __stringify(encap_type))
 
 #endif /* __NET_LWTUNNEL_H */
diff --git a/net/core/lwtunnel.c b/net/core/lwtunnel.c
index 299cfc24d888..8ef5e5cec03e 100644
--- a/net/core/lwtunnel.c
+++ b/net/core/lwtunnel.c
@@ -27,6 +27,30 @@
 #include <net/rtnetlink.h>
 #include <net/ip6_fib.h>
 
+#ifdef CONFIG_MODULES
+
+static const char *lwtunnel_encap_str(enum lwtunnel_encap_types encap_type)
+{
+	switch (encap_type) {
+	case LWTUNNEL_ENCAP_MPLS:
+		return "LWTUNNEL_ENCAP_MPLS";
+	case LWTUNNEL_ENCAP_IP:
+		return "LWTUNNEL_ENCAP_IP";
+	case LWTUNNEL_ENCAP_ILA:
+		return "LWTUNNEL_ENCAP_ILA";
+	case LWTUNNEL_ENCAP_IP6:
+		return "LWTUNNEL_ENCAP_IP6";
+	case LWTUNNEL_ENCAP_NONE:
+	case __LWTUNNEL_ENCAP_MAX:
+		/* should not have got here */
+		break;
+	}
+	WARN_ON(1);
+	return "LWTUNNEL_ENCAP_NONE";
+}
+
+#endif /* CONFIG_MODULES */
+
 struct lwtunnel_state *lwtunnel_state_alloc(int encap_len)
 {
 	struct lwtunnel_state *lws;
@@ -85,6 +109,14 @@ int lwtunnel_build_state(struct net_device *dev, u16 encap_type,
 	ret = -EOPNOTSUPP;
 	rcu_read_lock();
 	ops = rcu_dereference(lwtun_encaps[encap_type]);
+#ifdef CONFIG_MODULES
+	if (!ops) {
+		rcu_read_unlock();
+		request_module("rtnl-lwt-%s", lwtunnel_encap_str(encap_type));
+		rcu_read_lock();
+		ops = rcu_dereference(lwtun_encaps[encap_type]);
+	}
+#endif
 	if (likely(ops && ops->build_state))
 		ret = ops->build_state(dev, encap, family, cfg, lws);
 	rcu_read_unlock();
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH net-next 2/3] mpls: autoload lwt module
  2016-02-15 15:42 [PATCH net-next 0/3] lwtunnel: autoload of lwt modules Robert Shearman
  2016-02-15 15:42 ` [PATCH net-next 1/3] " Robert Shearman
@ 2016-02-15 15:42 ` Robert Shearman
  2016-02-15 15:42 ` [PATCH net-next 3/3] ila: autoload module Robert Shearman
  2016-02-19  9:43 ` [PATCH net-next v2 0/3] lwtunnel: autoload of lwt modules Robert Shearman
  3 siblings, 0 replies; 15+ messages in thread
From: Robert Shearman @ 2016-02-15 15:42 UTC (permalink / raw)
  To: davem
  Cc: netdev, Roopa Prabhu, Tom Herbert, Thomas Graf,
	Eric W. Biederman, Jiri Benc, Robert Shearman

Avoid users having to manually load the module by adding a module
alias allowing it to be autoloaded by the lwt infra.

Signed-off-by: Robert Shearman <rshearma@brocade.com>
---
 net/mpls/mpls_iptunnel.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/mpls/mpls_iptunnel.c b/net/mpls/mpls_iptunnel.c
index fb31aa87de81..1b4536960f79 100644
--- a/net/mpls/mpls_iptunnel.c
+++ b/net/mpls/mpls_iptunnel.c
@@ -227,5 +227,6 @@ static void __exit mpls_iptunnel_exit(void)
 }
 module_exit(mpls_iptunnel_exit);
 
+MODULE_ALIAS_RTNL_LWT(LWTUNNEL_ENCAP_MPLS);
 MODULE_DESCRIPTION("MultiProtocol Label Switching IP Tunnels");
 MODULE_LICENSE("GPL v2");
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH net-next 3/3] ila: autoload module
  2016-02-15 15:42 [PATCH net-next 0/3] lwtunnel: autoload of lwt modules Robert Shearman
  2016-02-15 15:42 ` [PATCH net-next 1/3] " Robert Shearman
  2016-02-15 15:42 ` [PATCH net-next 2/3] mpls: autoload lwt module Robert Shearman
@ 2016-02-15 15:42 ` Robert Shearman
  2016-02-19  9:43 ` [PATCH net-next v2 0/3] lwtunnel: autoload of lwt modules Robert Shearman
  3 siblings, 0 replies; 15+ messages in thread
From: Robert Shearman @ 2016-02-15 15:42 UTC (permalink / raw)
  To: davem
  Cc: netdev, Roopa Prabhu, Tom Herbert, Thomas Graf,
	Eric W. Biederman, Jiri Benc, Robert Shearman

Avoid users having to manually load the module by adding a module
alias allowing it to be autoloaded by the lwt infra.

Signed-off-by: Robert Shearman <rshearma@brocade.com>
---
 net/ipv6/ila/ila_common.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/ipv6/ila/ila_common.c b/net/ipv6/ila/ila_common.c
index 32dc9aab7297..8ecf2482560e 100644
--- a/net/ipv6/ila/ila_common.c
+++ b/net/ipv6/ila/ila_common.c
@@ -99,5 +99,6 @@ static void __exit ila_fini(void)
 
 module_init(ila_init);
 module_exit(ila_fini);
+MODULE_ALIAS_RTNL_LWT(LWTUNNEL_ENCAP_ILA);
 MODULE_AUTHOR("Tom Herbert <tom@herbertland.com>");
 MODULE_LICENSE("GPL");
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH net-next 1/3] lwtunnel: autoload of lwt modules
  2016-02-15 15:42 ` [PATCH net-next 1/3] " Robert Shearman
@ 2016-02-15 16:02   ` Jiri Benc
  2016-02-15 16:22     ` Robert Shearman
  2016-02-15 21:33   ` Eric W. Biederman
  1 sibling, 1 reply; 15+ messages in thread
From: Jiri Benc @ 2016-02-15 16:02 UTC (permalink / raw)
  To: Robert Shearman
  Cc: davem, netdev, Roopa Prabhu, Tom Herbert, Thomas Graf, Eric W. Biederman

On Mon, 15 Feb 2016 15:42:01 +0000, Robert Shearman wrote:
> +static const char *lwtunnel_encap_str(enum lwtunnel_encap_types encap_type)
> +{
> +	switch (encap_type) {
> +	case LWTUNNEL_ENCAP_MPLS:
> +		return "LWTUNNEL_ENCAP_MPLS";
> +	case LWTUNNEL_ENCAP_IP:
> +		return "LWTUNNEL_ENCAP_IP";
> +	case LWTUNNEL_ENCAP_ILA:
> +		return "LWTUNNEL_ENCAP_ILA";
> +	case LWTUNNEL_ENCAP_IP6:
> +		return "LWTUNNEL_ENCAP_IP6";
> +	case LWTUNNEL_ENCAP_NONE:
> +	case __LWTUNNEL_ENCAP_MAX:
> +		/* should not have got here */
> +		break;
> +	}
> +	WARN_ON(1);
> +	return "LWTUNNEL_ENCAP_NONE";
> +}
> +
> +#endif /* CONFIG_MODULES */
> +
>  struct lwtunnel_state *lwtunnel_state_alloc(int encap_len)
>  {
>  	struct lwtunnel_state *lws;
> @@ -85,6 +109,14 @@ int lwtunnel_build_state(struct net_device *dev, u16 encap_type,
>  	ret = -EOPNOTSUPP;
>  	rcu_read_lock();
>  	ops = rcu_dereference(lwtun_encaps[encap_type]);
> +#ifdef CONFIG_MODULES
> +	if (!ops) {
> +		rcu_read_unlock();
> +		request_module("rtnl-lwt-%s", lwtunnel_encap_str(encap_type));

Why the repeating of "lwt"/"LWTUNNEL" and the unnecessary "ENCAP"?
Wouldn't be lwtunnel_encap_str returning just "MPLS" or "ILA" enough?
I don't have any strong preference here, it just looks weird to me.
Maybe there's a reason.

Also, this doesn't affect IP lwtunnels, i.e. LWTUNNEL_ENCAP_IP and
LWTUNNEL_ENCAP_IP6. Should we just return NULL from lwtunnel_encap_str
in such cases (plus unknown encap_type) and WARN on the NULL here?

 Jiri

-- 
Jiri Benc

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH net-next 1/3] lwtunnel: autoload of lwt modules
  2016-02-15 16:02   ` Jiri Benc
@ 2016-02-15 16:22     ` Robert Shearman
  2016-02-15 16:32       ` Jiri Benc
  0 siblings, 1 reply; 15+ messages in thread
From: Robert Shearman @ 2016-02-15 16:22 UTC (permalink / raw)
  To: Jiri Benc
  Cc: davem, netdev, Roopa Prabhu, Tom Herbert, Thomas Graf, Eric W. Biederman

On 15/02/16 16:02, Jiri Benc wrote:
> On Mon, 15 Feb 2016 15:42:01 +0000, Robert Shearman wrote:
>> +static const char *lwtunnel_encap_str(enum lwtunnel_encap_types encap_type)
>> +{
>> +	switch (encap_type) {
>> +	case LWTUNNEL_ENCAP_MPLS:
>> +		return "LWTUNNEL_ENCAP_MPLS";
>> +	case LWTUNNEL_ENCAP_IP:
>> +		return "LWTUNNEL_ENCAP_IP";
>> +	case LWTUNNEL_ENCAP_ILA:
>> +		return "LWTUNNEL_ENCAP_ILA";
>> +	case LWTUNNEL_ENCAP_IP6:
>> +		return "LWTUNNEL_ENCAP_IP6";
>> +	case LWTUNNEL_ENCAP_NONE:
>> +	case __LWTUNNEL_ENCAP_MAX:
>> +		/* should not have got here */
>> +		break;
>> +	}
>> +	WARN_ON(1);
>> +	return "LWTUNNEL_ENCAP_NONE";
>> +}
>> +
>> +#endif /* CONFIG_MODULES */
>> +
>>   struct lwtunnel_state *lwtunnel_state_alloc(int encap_len)
>>   {
>>   	struct lwtunnel_state *lws;
>> @@ -85,6 +109,14 @@ int lwtunnel_build_state(struct net_device *dev, u16 encap_type,
>>   	ret = -EOPNOTSUPP;
>>   	rcu_read_lock();
>>   	ops = rcu_dereference(lwtun_encaps[encap_type]);
>> +#ifdef CONFIG_MODULES
>> +	if (!ops) {
>> +		rcu_read_unlock();
>> +		request_module("rtnl-lwt-%s", lwtunnel_encap_str(encap_type));
>
> Why the repeating of "lwt"/"LWTUNNEL" and the unnecessary "ENCAP"?
> Wouldn't be lwtunnel_encap_str returning just "MPLS" or "ILA" enough?
> I don't have any strong preference here, it just looks weird to me.
> Maybe there's a reason.

Yeah, it's the C preprocessor. MODULE_ALIAS_RTNL_LWT includes the string 
for the encap type in the module alias, and since the LWT encap types 
are defined as enums this is symbolic name. I can't see any way of 
getting the preprocessor to convert 
MODULE_ALIAS_RTNL_LWT(LWTUNNEL_ENCAP_MPLS) into "rtnl-lwt-MPLS", but I'm 
open to suggestions.

I could just drop the "lwt-" bit of the alias string given that it's 
included in the name of the enum values.

> Also, this doesn't affect IP lwtunnels, i.e. LWTUNNEL_ENCAP_IP and
> LWTUNNEL_ENCAP_IP6. Should we just return NULL from lwtunnel_encap_str
> in such cases (plus unknown encap_type) and WARN on the NULL here?

True, but I figured that it was cleaner for the lwtunnel infra to not 
assume whether how those modules are implemented. If you disagree, then 
I can change to doing as you suggest.

Thanks,
Rob

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH net-next 1/3] lwtunnel: autoload of lwt modules
  2016-02-15 16:22     ` Robert Shearman
@ 2016-02-15 16:32       ` Jiri Benc
  2016-02-15 18:08         ` Robert Shearman
  0 siblings, 1 reply; 15+ messages in thread
From: Jiri Benc @ 2016-02-15 16:32 UTC (permalink / raw)
  To: Robert Shearman
  Cc: davem, netdev, Roopa Prabhu, Tom Herbert, Thomas Graf, Eric W. Biederman

On Mon, 15 Feb 2016 16:22:08 +0000, Robert Shearman wrote:
> Yeah, it's the C preprocessor. MODULE_ALIAS_RTNL_LWT includes the string 
> for the encap type in the module alias, and since the LWT encap types 
> are defined as enums this is symbolic name. I can't see any way of 
> getting the preprocessor to convert 
> MODULE_ALIAS_RTNL_LWT(LWTUNNEL_ENCAP_MPLS) into "rtnl-lwt-MPLS", but I'm 
> open to suggestions.

MODULE_ALIAS_RTNL_LWT(MPLS)?

But whatever, as I said, no strong preference.

> True, but I figured that it was cleaner for the lwtunnel infra to not 
> assume whether how those modules are implemented. If you disagree, then 
> I can change to doing as you suggest.

It's not completely transparent to the infrastructure anyway, the
tunnel type needs to be added to lwtunnel_encap_str for new tunnels.
The way I suggested, it's only added for those tunnels having the
module alias set.

Just trying to get rid of the unnecessary strings in
lwtunnel_encap_str. There's no need to bloat kernel with them if
they're never used.

 Jiri

-- 
Jiri Benc

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH net-next 1/3] lwtunnel: autoload of lwt modules
  2016-02-15 16:32       ` Jiri Benc
@ 2016-02-15 18:08         ` Robert Shearman
  0 siblings, 0 replies; 15+ messages in thread
From: Robert Shearman @ 2016-02-15 18:08 UTC (permalink / raw)
  To: Jiri Benc
  Cc: davem, netdev, Roopa Prabhu, Tom Herbert, Thomas Graf, Eric W. Biederman

On 15/02/16 16:32, Jiri Benc wrote:
> On Mon, 15 Feb 2016 16:22:08 +0000, Robert Shearman wrote:
>> Yeah, it's the C preprocessor. MODULE_ALIAS_RTNL_LWT includes the string
>> for the encap type in the module alias, and since the LWT encap types
>> are defined as enums this is symbolic name. I can't see any way of
>> getting the preprocessor to convert
>> MODULE_ALIAS_RTNL_LWT(LWTUNNEL_ENCAP_MPLS) into "rtnl-lwt-MPLS", but I'm
>> open to suggestions.
>
> MODULE_ALIAS_RTNL_LWT(MPLS)?
>
> But whatever, as I said, no strong preference.

I was so hung up on the making the string match the name of the enum 
that I'd discounted that, but you're right that doing that would reduce 
duplication in the alias string.

>> True, but I figured that it was cleaner for the lwtunnel infra to not
>> assume whether how those modules are implemented. If you disagree, then
>> I can change to doing as you suggest.
>
> It's not completely transparent to the infrastructure anyway, the
> tunnel type needs to be added to lwtunnel_encap_str for new tunnels.
> The way I suggested, it's only added for those tunnels having the
> module alias set.
>
> Just trying to get rid of the unnecessary strings in
> lwtunnel_encap_str. There's no need to bloat kernel with them if
> they're never used.

Ok, will resubmit without the unnecessary strings in that function as 
well as with your suggestion above.

Thanks for the review,
Rob

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH net-next 1/3] lwtunnel: autoload of lwt modules
  2016-02-15 15:42 ` [PATCH net-next 1/3] " Robert Shearman
  2016-02-15 16:02   ` Jiri Benc
@ 2016-02-15 21:33   ` Eric W. Biederman
  2016-02-16 14:14     ` Robert Shearman
  1 sibling, 1 reply; 15+ messages in thread
From: Eric W. Biederman @ 2016-02-15 21:33 UTC (permalink / raw)
  To: Robert Shearman
  Cc: davem, netdev, Roopa Prabhu, Tom Herbert, Thomas Graf, Jiri Benc

Robert Shearman <rshearma@brocade.com> writes:

> The lwt implementations using net devices can autoload using the
> existing mechanism using IFLA_INFO_KIND. However, there's no mechanism
> that lwt modules not using net devices can use.
>
> Therefore, add the ability to autoload modules registering lwt
> operations for lwt implementations not using a net device so that
> users don't have to manually load the modules.
>
> Signed-off-by: Robert Shearman <rshearma@brocade.com>
> ---
>  include/net/lwtunnel.h |  4 +++-
>  net/core/lwtunnel.c    | 32 ++++++++++++++++++++++++++++++++
>  2 files changed, 35 insertions(+), 1 deletion(-)
>
> diff --git a/include/net/lwtunnel.h b/include/net/lwtunnel.h
> index 66350ce3e955..e9f116e29c22 100644
> --- a/include/net/lwtunnel.h
> +++ b/include/net/lwtunnel.h
> @@ -170,6 +170,8 @@ static inline int lwtunnel_input(struct sk_buff *skb)
>  	return -EOPNOTSUPP;
>  }
>  
> -#endif
> +#endif /* CONFIG_LWTUNNEL */
> +
> +#define MODULE_ALIAS_RTNL_LWT(encap_type) MODULE_ALIAS("rtnl-lwt-" __stringify(encap_type))
>  
>  #endif /* __NET_LWTUNNEL_H */
> diff --git a/net/core/lwtunnel.c b/net/core/lwtunnel.c
> index 299cfc24d888..8ef5e5cec03e 100644
> --- a/net/core/lwtunnel.c
> +++ b/net/core/lwtunnel.c
> @@ -27,6 +27,30 @@
>  #include <net/rtnetlink.h>
>  #include <net/ip6_fib.h>
>  
> +#ifdef CONFIG_MODULES
> +
> +static const char *lwtunnel_encap_str(enum lwtunnel_encap_types encap_type)
> +{
> +	switch (encap_type) {
> +	case LWTUNNEL_ENCAP_MPLS:
> +		return "LWTUNNEL_ENCAP_MPLS";
> +	case LWTUNNEL_ENCAP_IP:
> +		return "LWTUNNEL_ENCAP_IP";
> +	case LWTUNNEL_ENCAP_ILA:
> +		return "LWTUNNEL_ENCAP_ILA";
> +	case LWTUNNEL_ENCAP_IP6:
> +		return "LWTUNNEL_ENCAP_IP6";
> +	case LWTUNNEL_ENCAP_NONE:
> +	case __LWTUNNEL_ENCAP_MAX:
> +		/* should not have got here */
> +		break;
> +	}
> +	WARN_ON(1);
> +	return "LWTUNNEL_ENCAP_NONE";
> +}
> +
> +#endif /* CONFIG_MODULES */
> +
>  struct lwtunnel_state *lwtunnel_state_alloc(int encap_len)
>  {
>  	struct lwtunnel_state *lws;
> @@ -85,6 +109,14 @@ int lwtunnel_build_state(struct net_device *dev, u16 encap_type,
>  	ret = -EOPNOTSUPP;
>  	rcu_read_lock();
>  	ops = rcu_dereference(lwtun_encaps[encap_type]);
> +#ifdef CONFIG_MODULES
> +	if (!ops) {
> +		rcu_read_unlock();
> +		request_module("rtnl-lwt-%s", lwtunnel_encap_str(encap_type));
> +		rcu_read_lock();
> +		ops = rcu_dereference(lwtun_encaps[encap_type]);
> +	}
> +#endif
>  	if (likely(ops && ops->build_state))
>  		ret = ops->build_state(dev, encap, family, cfg, lws);
>  	rcu_read_unlock();

My memory is fuzzy on how this is done elsewhere but this looks like it
needs a capability check to ensure that non-root user's can't trigger
this.

It tends to be problematic if a non-root user can trigger an autoload of
a known-buggy module.  With a combination of user namespaces and network
namespaces unprivileged users can cause just about every corner of the
network stack to be exercised.

Eric

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH net-next 1/3] lwtunnel: autoload of lwt modules
  2016-02-15 21:33   ` Eric W. Biederman
@ 2016-02-16 14:14     ` Robert Shearman
  0 siblings, 0 replies; 15+ messages in thread
From: Robert Shearman @ 2016-02-16 14:14 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: davem, netdev, Roopa Prabhu, Tom Herbert, Thomas Graf, Jiri Benc

On 15/02/16 21:33, Eric W. Biederman wrote:
> Robert Shearman <rshearma@brocade.com> writes:
>> @@ -85,6 +109,14 @@ int lwtunnel_build_state(struct net_device *dev, u16 encap_type,
>>   	ret = -EOPNOTSUPP;
>>   	rcu_read_lock();
>>   	ops = rcu_dereference(lwtun_encaps[encap_type]);
>> +#ifdef CONFIG_MODULES
>> +	if (!ops) {
>> +		rcu_read_unlock();
>> +		request_module("rtnl-lwt-%s", lwtunnel_encap_str(encap_type));
>> +		rcu_read_lock();
>> +		ops = rcu_dereference(lwtun_encaps[encap_type]);
>> +	}
>> +#endif
>>   	if (likely(ops && ops->build_state))
>>   		ret = ops->build_state(dev, encap, family, cfg, lws);
>>   	rcu_read_unlock();
>
> My memory is fuzzy on how this is done elsewhere but this looks like it
> needs a capability check to ensure that non-root user's can't trigger
> this.
>
> It tends to be problematic if a non-root user can trigger an autoload of
> a known-buggy module.  With a combination of user namespaces and network
> namespaces unprivileged users can cause just about every corner of the
> network stack to be exercised.

The same protections apply to this as to the IFLA_INFO_KIND module 
autoloading, namely by rtnetlink_rcv_msg ensuring that no messages other 
than gets can be done by an unprivileged user:

	type = nlh->nlmsg_type;
...
	type -= RTM_BASE;
...
	kind = type&3;

	if (kind != 2 && !netlink_net_capable(skb, CAP_NET_ADMIN))
		return -EPERM;

The lwtunnel_build_state function is only called by the processing of 
non-get message types.

Is this sufficient or are you looking for something in addition?

Thanks,
Rob

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH net-next v2 0/3] lwtunnel: autoload of lwt modules
  2016-02-15 15:42 [PATCH net-next 0/3] lwtunnel: autoload of lwt modules Robert Shearman
                   ` (2 preceding siblings ...)
  2016-02-15 15:42 ` [PATCH net-next 3/3] ila: autoload module Robert Shearman
@ 2016-02-19  9:43 ` Robert Shearman
  2016-02-19  9:43   ` [PATCH net-next v2 1/3] " Robert Shearman
                     ` (3 more replies)
  3 siblings, 4 replies; 15+ messages in thread
From: Robert Shearman @ 2016-02-19  9:43 UTC (permalink / raw)
  To: davem
  Cc: netdev, Roopa Prabhu, Tom Herbert, Thomas Graf,
	Eric W. Biederman, Jiri Benc, Robert Shearman

Changes since v1:
 - remove "LWTUNNEL_ENCAP_" prefix for the string form of the encaps
   used when requesting the module to reduce duplication, and don't
   bother returning strings for lwt modules using netdevices, both
   suggested by Jiri.
 - update commit message of first patch to clarify security
   implications, in response to Eric's comments.

The lwt implementations using net devices can autoload using the
existing mechanism using IFLA_INFO_KIND. However, there's no mechanism
that lwt modules not using net devices can use.

Therefore, these patches add the ability to autoload modules
registering lwt operations for lwt implementations not using a net
device so that users don't have to manually load the modules.

Robert Shearman (3):
  lwtunnel: autoload of lwt modules
  mpls: autoload lwt module
  ila: autoload module

 include/net/lwtunnel.h    |  4 +++-
 net/core/lwtunnel.c       | 37 +++++++++++++++++++++++++++++++++++++
 net/ipv6/ila/ila_common.c |  1 +
 net/mpls/mpls_iptunnel.c  |  1 +
 4 files changed, 42 insertions(+), 1 deletion(-)

-- 
2.1.4

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH net-next v2 1/3] lwtunnel: autoload of lwt modules
  2016-02-19  9:43 ` [PATCH net-next v2 0/3] lwtunnel: autoload of lwt modules Robert Shearman
@ 2016-02-19  9:43   ` Robert Shearman
  2016-02-19  9:43   ` [PATCH net-next v2 2/3] mpls: autoload lwt module Robert Shearman
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 15+ messages in thread
From: Robert Shearman @ 2016-02-19  9:43 UTC (permalink / raw)
  To: davem
  Cc: netdev, Roopa Prabhu, Tom Herbert, Thomas Graf,
	Eric W. Biederman, Jiri Benc, Robert Shearman

The lwt implementations using net devices can autoload using the
existing mechanism using IFLA_INFO_KIND. However, there's no mechanism
that lwt modules not using net devices can use.

Therefore, add the ability to autoload modules registering lwt
operations for lwt implementations not using a net device so that
users don't have to manually load the modules.

Only users with the CAP_NET_ADMIN capability can cause modules to be
loaded, which is ensured by rtnetlink_rcv_msg rejecting non-RTM_GETxxx
messages for users without this capability, and by
lwtunnel_build_state not being called in response to RTM_GETxxx
messages.

Signed-off-by: Robert Shearman <rshearma@brocade.com>
---
 include/net/lwtunnel.h |  4 +++-
 net/core/lwtunnel.c    | 37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/include/net/lwtunnel.h b/include/net/lwtunnel.h
index 66350ce3e955..e9f116e29c22 100644
--- a/include/net/lwtunnel.h
+++ b/include/net/lwtunnel.h
@@ -170,6 +170,8 @@ static inline int lwtunnel_input(struct sk_buff *skb)
 	return -EOPNOTSUPP;
 }
 
-#endif
+#endif /* CONFIG_LWTUNNEL */
+
+#define MODULE_ALIAS_RTNL_LWT(encap_type) MODULE_ALIAS("rtnl-lwt-" __stringify(encap_type))
 
 #endif /* __NET_LWTUNNEL_H */
diff --git a/net/core/lwtunnel.c b/net/core/lwtunnel.c
index 299cfc24d888..669ecc9f884e 100644
--- a/net/core/lwtunnel.c
+++ b/net/core/lwtunnel.c
@@ -27,6 +27,31 @@
 #include <net/rtnetlink.h>
 #include <net/ip6_fib.h>
 
+#ifdef CONFIG_MODULES
+
+static const char *lwtunnel_encap_str(enum lwtunnel_encap_types encap_type)
+{
+	/* Only lwt encaps implemented without using an interface for
+	 * the encap need to return a string here.
+	 */
+	switch (encap_type) {
+	case LWTUNNEL_ENCAP_MPLS:
+		return "MPLS";
+	case LWTUNNEL_ENCAP_ILA:
+		return "ILA";
+	case LWTUNNEL_ENCAP_IP6:
+	case LWTUNNEL_ENCAP_IP:
+	case LWTUNNEL_ENCAP_NONE:
+	case __LWTUNNEL_ENCAP_MAX:
+		/* should not have got here */
+		WARN_ON(1);
+		break;
+	}
+	return NULL;
+}
+
+#endif /* CONFIG_MODULES */
+
 struct lwtunnel_state *lwtunnel_state_alloc(int encap_len)
 {
 	struct lwtunnel_state *lws;
@@ -85,6 +110,18 @@ int lwtunnel_build_state(struct net_device *dev, u16 encap_type,
 	ret = -EOPNOTSUPP;
 	rcu_read_lock();
 	ops = rcu_dereference(lwtun_encaps[encap_type]);
+#ifdef CONFIG_MODULES
+	if (!ops) {
+		const char *encap_type_str = lwtunnel_encap_str(encap_type);
+
+		if (encap_type_str) {
+			rcu_read_unlock();
+			request_module("rtnl-lwt-%s", encap_type_str);
+			rcu_read_lock();
+			ops = rcu_dereference(lwtun_encaps[encap_type]);
+		}
+	}
+#endif
 	if (likely(ops && ops->build_state))
 		ret = ops->build_state(dev, encap, family, cfg, lws);
 	rcu_read_unlock();
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH net-next v2 2/3] mpls: autoload lwt module
  2016-02-19  9:43 ` [PATCH net-next v2 0/3] lwtunnel: autoload of lwt modules Robert Shearman
  2016-02-19  9:43   ` [PATCH net-next v2 1/3] " Robert Shearman
@ 2016-02-19  9:43   ` Robert Shearman
  2016-02-19  9:43   ` [PATCH net-next v2 3/3] ila: autoload module Robert Shearman
  2016-02-22  3:00   ` [PATCH net-next v2 0/3] lwtunnel: autoload of lwt modules David Miller
  3 siblings, 0 replies; 15+ messages in thread
From: Robert Shearman @ 2016-02-19  9:43 UTC (permalink / raw)
  To: davem
  Cc: netdev, Roopa Prabhu, Tom Herbert, Thomas Graf,
	Eric W. Biederman, Jiri Benc, Robert Shearman

Avoid users having to manually load the module by adding a module
alias allowing it to be autoloaded by the lwt infra.

Signed-off-by: Robert Shearman <rshearma@brocade.com>
---
 net/mpls/mpls_iptunnel.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/mpls/mpls_iptunnel.c b/net/mpls/mpls_iptunnel.c
index fb31aa87de81..644a8da6d4bd 100644
--- a/net/mpls/mpls_iptunnel.c
+++ b/net/mpls/mpls_iptunnel.c
@@ -227,5 +227,6 @@ static void __exit mpls_iptunnel_exit(void)
 }
 module_exit(mpls_iptunnel_exit);
 
+MODULE_ALIAS_RTNL_LWT(MPLS);
 MODULE_DESCRIPTION("MultiProtocol Label Switching IP Tunnels");
 MODULE_LICENSE("GPL v2");
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH net-next v2 3/3] ila: autoload module
  2016-02-19  9:43 ` [PATCH net-next v2 0/3] lwtunnel: autoload of lwt modules Robert Shearman
  2016-02-19  9:43   ` [PATCH net-next v2 1/3] " Robert Shearman
  2016-02-19  9:43   ` [PATCH net-next v2 2/3] mpls: autoload lwt module Robert Shearman
@ 2016-02-19  9:43   ` Robert Shearman
  2016-02-22  3:00   ` [PATCH net-next v2 0/3] lwtunnel: autoload of lwt modules David Miller
  3 siblings, 0 replies; 15+ messages in thread
From: Robert Shearman @ 2016-02-19  9:43 UTC (permalink / raw)
  To: davem
  Cc: netdev, Roopa Prabhu, Tom Herbert, Thomas Graf,
	Eric W. Biederman, Jiri Benc, Robert Shearman

Avoid users having to manually load the module by adding a module
alias allowing it to be autoloaded by the lwt infra.

Signed-off-by: Robert Shearman <rshearma@brocade.com>
---
 net/ipv6/ila/ila_common.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/ipv6/ila/ila_common.c b/net/ipv6/ila/ila_common.c
index 32dc9aab7297..30613050e4ca 100644
--- a/net/ipv6/ila/ila_common.c
+++ b/net/ipv6/ila/ila_common.c
@@ -99,5 +99,6 @@ static void __exit ila_fini(void)
 
 module_init(ila_init);
 module_exit(ila_fini);
+MODULE_ALIAS_RTNL_LWT(ILA);
 MODULE_AUTHOR("Tom Herbert <tom@herbertland.com>");
 MODULE_LICENSE("GPL");
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH net-next v2 0/3] lwtunnel: autoload of lwt modules
  2016-02-19  9:43 ` [PATCH net-next v2 0/3] lwtunnel: autoload of lwt modules Robert Shearman
                     ` (2 preceding siblings ...)
  2016-02-19  9:43   ` [PATCH net-next v2 3/3] ila: autoload module Robert Shearman
@ 2016-02-22  3:00   ` David Miller
  3 siblings, 0 replies; 15+ messages in thread
From: David Miller @ 2016-02-22  3:00 UTC (permalink / raw)
  To: rshearma; +Cc: netdev, roopa, tom, tgraf, ebiederm, jbenc

From: Robert Shearman <rshearma@brocade.com>
Date: Fri, 19 Feb 2016 09:43:15 +0000

> Changes since v1:
>  - remove "LWTUNNEL_ENCAP_" prefix for the string form of the encaps
>    used when requesting the module to reduce duplication, and don't
>    bother returning strings for lwt modules using netdevices, both
>    suggested by Jiri.
>  - update commit message of first patch to clarify security
>    implications, in response to Eric's comments.
> 
> The lwt implementations using net devices can autoload using the
> existing mechanism using IFLA_INFO_KIND. However, there's no mechanism
> that lwt modules not using net devices can use.
> 
> Therefore, these patches add the ability to autoload modules
> registering lwt operations for lwt implementations not using a net
> device so that users don't have to manually load the modules.

Series applied, thanks.

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2016-02-22  3:00 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-15 15:42 [PATCH net-next 0/3] lwtunnel: autoload of lwt modules Robert Shearman
2016-02-15 15:42 ` [PATCH net-next 1/3] " Robert Shearman
2016-02-15 16:02   ` Jiri Benc
2016-02-15 16:22     ` Robert Shearman
2016-02-15 16:32       ` Jiri Benc
2016-02-15 18:08         ` Robert Shearman
2016-02-15 21:33   ` Eric W. Biederman
2016-02-16 14:14     ` Robert Shearman
2016-02-15 15:42 ` [PATCH net-next 2/3] mpls: autoload lwt module Robert Shearman
2016-02-15 15:42 ` [PATCH net-next 3/3] ila: autoload module Robert Shearman
2016-02-19  9:43 ` [PATCH net-next v2 0/3] lwtunnel: autoload of lwt modules Robert Shearman
2016-02-19  9:43   ` [PATCH net-next v2 1/3] " Robert Shearman
2016-02-19  9:43   ` [PATCH net-next v2 2/3] mpls: autoload lwt module Robert Shearman
2016-02-19  9:43   ` [PATCH net-next v2 3/3] ila: autoload module Robert Shearman
2016-02-22  3:00   ` [PATCH net-next v2 0/3] lwtunnel: autoload of lwt modules David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).