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
| ||
|
Date: Sun, 14 May 2017 08:57:34 -0500 From: ebiederm@...ssion.com (Eric W. Biederman) To: Greg Kroah-Hartman <gregkh@...uxfoundation.org> Cc: Mahesh Bandewar <mahesh@...dewar.net>, Ingo Molnar <mingo@...nel.org>, LKML <linux-kernel@...r.kernel.org>, netdev <netdev@...r.kernel.org>, Kees Cook <keescook@...omium.org>, David Miller <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Mahesh Bandewar <maheshb@...gle.com> Subject: Re: [PATCH] kmod: don't load module unless req process has CAP_SYS_MODULE Greg Kroah-Hartman <gregkh@...uxfoundation.org> writes: > On Fri, May 12, 2017 at 04:22:59PM -0700, Mahesh Bandewar wrote: >> From: Mahesh Bandewar <maheshb@...gle.com> >> >> A process inside random user-ns should not load a module, which is >> currently possible. As demonstrated in following scenario - >> >> Create namespaces; especially a user-ns and become root inside. >> $ unshare -rfUp -- unshare -unm -- bash >> >> Try to load the bridge module. It should fail and this is expected! >> # modprobe bridge >> WARNING: Error inserting stp (/lib/modules/4.11.0-smp-DEV/kernel/net/802/stp.ko): Operation not permitted >> FATAL: Error inserting bridge (/lib/modules/4.11.0-smp-DEV/kernel/net/bridge/bridge.ko): Operation not permitted >> >> Verify bridge module is not loaded. >> # lsmod | grep bridge >> # >> >> Now try to create a bridge inside this newly created net-ns which would >> mean bridge module need to be loaded. >> # ip link add br0 type bridge >> # echo $? >> 0 >> # lsmod | grep bridge >> bridge 110592 0 >> stp 16384 1 bridge >> llc 16384 2 bridge,stp >> # >> >> After this patch - >> # ip link add br0 type bridge >> RTNETLINK answers: Operation not supported >> # echo $? >> 2 >> # lsmod | grep bridge >> # > > Well, it only loads this because the kernel asked for it to be loaded, > right? > >> >> Signed-off-by: Mahesh Bandewar <maheshb@...gle.com> >> --- >> kernel/kmod.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/kernel/kmod.c b/kernel/kmod.c >> index 563f97e2be36..ac30157169b7 100644 >> --- a/kernel/kmod.c >> +++ b/kernel/kmod.c >> @@ -133,6 +133,9 @@ int __request_module(bool wait, const char *fmt, ...) >> #define MAX_KMOD_CONCURRENT 50 /* Completely arbitrary value - KAO */ >> static int kmod_loop_msg; >> >> + if (!capable(CAP_SYS_MODULE)) >> + return -EPERM; > > At first glance this looks right, but I'm worried what this will break > that currently relies on this. There might be lots of systems that are > used to this being the method that the needed module is requested. What > about when userspace asks for a random char device and that module is > then loaded? Does this patch break that functionality? For the specific example give I think we would be better served by adding a capability check at the call site. In this case CAP_NET_ADMIN as those are the capabilities iproute traditionally has. We have something similar in dev_load in already in the networking code. This limits the people who can't load modules to root user in user namespaces. I would be fine with any other code paths in a user namespace getting a similar treatment. Eric diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c index bcb0f610ee42..6b72528a4636 100644 --- a/net/core/rtnetlink.c +++ b/net/core/rtnetlink.c @@ -2595,7 +2595,7 @@ static int rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh, if (!ops) { #ifdef CONFIG_MODULES - if (kind[0]) { + if (kind[0] && capable(CAP_NET_ADMIN)) { __rtnl_unlock(); request_module("rtnl-link-%s", kind); rtnl_lock();
Powered by blists - more mailing lists