On Sun, May 14, 2017 at 3:45 AM, Greg Kroah-Hartman <gre...@linuxfoundation.org> wrote: > On Fri, May 12, 2017 at 04:22:59PM -0700, Mahesh Bandewar wrote: >> From: Mahesh Bandewar <mahe...@google.com> >> [...] >> 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? > Yes, kernel asked for it because of a user action.
>> >> Signed-off-by: Mahesh Bandewar <mahe...@google.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? > Any module when loaded gets loaded system-wide as we can't allow module loading per-ns. To validate the behavior I was comparing it with insmod/modprobe, if that doesn't allow because of lack of this capability in default-ns, then this *indirect* method of loading module should not allow the same action and the behavior should be consistent. So with that logic if userspace asks for a random char-device if insmod/modprobe cannot load it, then this method should not load it either for the consistency, right? > thanks, > > greg k-h
