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
| ||
|
Message-ID: <CAGXu5j+UkB0UgFKOHXZ3T=2EGk54soemtLuAuv3AdyX2nZ=Y=A@mail.gmail.com> Date: Mon, 15 May 2017 10:07:27 -0700 From: Kees Cook <keescook@...omium.org> To: Eric Dumazet <edumazet@...gle.com> Cc: Mahesh Bandewar (महेश बंडेवार) <maheshb@...gle.com>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Mahesh Bandewar <mahesh@...dewar.net>, Ingo Molnar <mingo@...nel.org>, LKML <linux-kernel@...r.kernel.org>, netdev <netdev@...r.kernel.org>, "Eric W . Biederman" <ebiederm@...ssion.com>, David Miller <davem@...emloft.net> Subject: Re: [PATCH] kmod: don't load module unless req process has CAP_SYS_MODULE On Mon, May 15, 2017 at 6:12 AM, Eric Dumazet <edumazet@...gle.com> wrote: > On Sun, May 14, 2017 at 7:42 PM, Mahesh Bandewar (महेश बंडेवार) > <maheshb@...gle.com> wrote: >> On Sun, May 14, 2017 at 3:45 AM, Greg Kroah-Hartman >> <gregkh@...uxfoundation.org> wrote: >>> On Fri, May 12, 2017 at 04:22:59PM -0700, Mahesh Bandewar wrote: >>>> From: Mahesh Bandewar <maheshb@...gle.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 <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? >>> >> 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? > > > This patch will break applications that expected modules being auto loaded. I would prefer that we continue to look at the autoloading restrictions series, since that will be more flexible and cover a wider set of cases: https://lkml.org/lkml/2017/4/19/1086 -Kees -- Kees Cook Pixel Security
Powered by blists - more mailing lists