Skip to contentSkip to conversation

add missing module_alias_subsys

PATCH In review Pablo Neira 3 participants 6 messages Nov 4, 2005 – Nov 5, 2005

Subsystems (2)
Networking netfilter
lore
Share & tools
lore

Pablo Neira

author maintainer
#0 lore raw

PATCH 4/7 add missing module_alias_subsys

Add missing module alias. This is a must to load ctnetlink on demand. For example, the conntrack tool will fail if the module isn't loaded.

Signature · 2 lines
The dawn of the fourth age of Linux firewalling is coming; a time of
great struggle and heroic deeds -- J.Kadlecsik got inspired by J.Morris

Add missing module alias. This is a must to load ctnetlink on demand. For example, the conntrack tool will fail if the module isn't loaded.

Signed-off-by Pablo Neira Ayuso <[email protected]>
net/ipv4/netfilter/ip_conntrack_netlink.c +2 -0
Index: net-2.6.git/net/ipv4/netfilter/ip_conntrack_netlink.c
===================================================================
@@ -1538,6 +1538,8 @@ static struct nfnetlink_subsystem ctnl_e
 	.cb				= ctnl_exp_cb,
 };
 
+MODULE_ALIAS_NFNL_SUBSYS(NFNL_SUBSYS_CTNETLINK);
+
 static int __init ctnetlink_init(void)
 {
 	int ret;
#1 lore raw
Quoted 2 lines · Pablo Neira

Add missing module alias. This is a must to load ctnetlink on demand. For example, the conntrack tool will fail if the module isn't loaded.

I don't think this is a good idea currently. Capability checking is done after module autoloading, so any user can load ctnetlink, ip_conntrack and all related modules. Please make sure to move capability checking in nfnetlink before module loading first.

BTW: The same applies to ip_tables and ipt_ULOG through netlink autoloading.

#2 lore raw
Quoted 7 lines · Patrick McHardy
Quoted 2 lines · Pablo Neira

Add missing module alias. This is a must to load ctnetlink on demand. For example, the conntrack tool will fail if the module isn't loaded.

I don't think this is a good idea currently. Capability checking is done after module autoloading, so any user can load ctnetlink, ip_conntrack and all related modules.

interesting point, thanks for mentioning it.

Quoted 2 lines

Please make sure to move capability checking in nfnetlink before module loading first.

This unfortunately doesn't work with the current architecture, where every nfnetlink subsystem can specifiy the required capabilities per message. That specification isn't available before loading the module, though.

I think we can (in addition to our usual capability checks) add a capability check to only do autoloading of a module if CAP_NET_ADMIN is set. Like:

[NETFILTER] nfnetlink: only load subsystems if CAP_NET_ADMIN is set

Without this patch, any user can cause nfnetlink subsystems to be autoloaded. Those subsystems however could add significant processing overhead to packet processing, and would refuse any configuration messages from non-CAP_NET_ADMIN processes anyway.

This patch follows a suggestion from Patrick McHardy.

Signed-off-by Harald Welte <[email protected]>

---
commit b73cccaf3b2d71b8f516c9b28cd44bf3b4efab1c
tree a67a2728bae003a7974ed9e5fd69bc8a40fd7f03
parent 5bd49cc95f4f8ef5e6782242709927cd3ee6337d
author Harald Welte <[email protected]> Sat, 05 Nov 2005 12:52:07 +0100 committer Harald Welte <[email protected]> Sat, 05 Nov 2005 12:52:07 +0100

 net/netfilter/nfnetlink.c |   17 ++++++++++-------
 1 files changed, 10 insertions(+), 7 deletions(-)
net/netfilter/nfnetlink.c +10 -7
@@ -240,15 +240,18 @@ static inline int nfnetlink_rcv_msg(stru
 	ss = nfnetlink_get_subsys(type);
 	if (!ss) {
 #ifdef CONFIG_KMOD
-		/* don't call nfnl_shunlock, since it would reenter
-		 * with further packet processing */
-		up(&nfnl_sem);
-		request_module("nfnetlink-subsys-%d", NFNL_SUBSYS_ID(type));
-		nfnl_shlock();
-		ss = nfnetlink_get_subsys(type);
+		if (cap_raised(NETLINK_CB(skb).eff_cap, CAP_NET_ADMIN)) {
+			/* don't call nfnl_shunlock, since it would reenter
+			 * with further packet processing */
+			up(&nfnl_sem);
+			request_module("nfnetlink-subsys-%d",
+					NFNL_SUBSYS_ID(type));
+			nfnl_shlock();
+			ss = nfnetlink_get_subsys(type);
+		}
 		if (!ss)
 #endif
-		goto err_inval;
+			goto err_inval;
 	}
 
 	nc = nfnetlink_find_client(type, ss);
Signature · 5 lines
- Harald Welte <[email protected]>                 http://netfilter.org/
============================================================================
  "Fragmentation is like classful addressing -- an interesting early
   architectural error that shows how much experimentation was going
   on while IP was being designed."                    -- Paul Vixie
#3 lore raw
Quoted 14 lines · Harald Welte
Quoted 8 lines · Patrick McHardy
Quoted 2 lines · Pablo Neira

Add missing module alias. This is a must to load ctnetlink on demand. For example, the conntrack tool will fail if the module isn't loaded.

I don't think this is a good idea currently. Capability checking is done after module autoloading, so any user can load ctnetlink, ip_conntrack and all related modules.

interesting point, thanks for mentioning it.

Quoted 8 lines
Quoted 2 lines

Please make sure to move capability checking in nfnetlink before module loading first.

This unfortunately doesn't work with the current architecture, where every nfnetlink subsystem can specifiy the required capabilities per message. That specification isn't available before loading the module, though.

Didn't we decide to remove the per-subsys capabilities and make all of them require CAP_NET_ADMIN?

Quoted 3 lines

I think we can (in addition to our usual capability checks) add a capability check to only do autoloading of a module if CAP_NET_ADMIN is set. Like:

That also a possiblity, but I can't think of a case where we wouldn't insist on CAP_NET_ADMIN, so just removing the whole per-subsys capabilities seems easier to me.

#4 lore raw
Quoted 7 lines · Patrick McHardy
Quoted 4 lines

This unfortunately doesn't work with the current architecture, where every nfnetlink subsystem can specifiy the required capabilities per message. That specification isn't available before loading the module, though.

Didn't we decide to remove the per-subsys capabilities and make all of them require CAP_NET_ADMIN?

We did? I knew we were talking about it, but I must have forgotten the result, sorry.

Quoted 3 lines

That also a possiblity, but I can't think of a case where we wouldn't insist on CAP_NET_ADMIN, so just removing the whole per-subsys capabilities seems easier to me.

Mh, ok. But you owe me a beer if we ever need to re-introduce it ;)

Signature · 5 lines
- Harald Welte <[email protected]>                 http://netfilter.org/
============================================================================
  "Fragmentation is like classful addressing -- an interesting early
   architectural error that shows how much experimentation was going
   on while IP was being designed."                    -- Paul Vixie
#5 lore raw
Quoted 8 lines · Harald Welte
Quoted 5 lines · Patrick McHardy
Quoted 4 lines

This unfortunately doesn't work with the current architecture, where every nfnetlink subsystem can specifiy the required capabilities per message. That specification isn't available before loading the module, though.

Quoted 6 lines
Quoted 2 lines

Didn't we decide to remove the per-subsys capabilities and make all of them require CAP_NET_ADMIN?

We did? I knew we were talking about it, but I must have forgotten the result, sorry.

I think we did in Montreal.

Quoted 5 lines
Quoted 3 lines

That also a possiblity, but I can't think of a case where we wouldn't insist on CAP_NET_ADMIN, so just removing the whole per-subsys capabilities seems easier to me.

Mh, ok. But you owe me a beer if we ever need to re-introduce it ;)

Agreed :)