Linux userland API discussions
 help / color / mirror / Atom feed
* Re: [PATCH] mmap.2: document new MAP_FIXED_NOREPLACE flag
From: John Hubbard @ 2018-04-11 20:36 UTC (permalink / raw)
  To: Jann Horn, Michal Hocko
  Cc: Michael Kerrisk, Andrew Morton, Linux-MM, LKML, Linux API,
	Michal Hocko
In-Reply-To: <CAG48ez3BS5EtnrhFQUGYY9MKGOUHzFbhauJQd361uTwy2pBEeg@mail.gmail.com>

On 04/11/2018 08:37 AM, Jann Horn wrote:
> On Wed, Apr 11, 2018 at 2:04 PM,  <mhocko@kernel.org> wrote:
>> From: Michal Hocko <mhocko@suse.com>
>>
>> 4.17+ kernels offer a new MAP_FIXED_NOREPLACE flag which allows the caller to
>> atomicaly probe for a given address range.
>>
>> [wording heavily updated by John Hubbard <jhubbard@nvidia.com>]
>> Signed-off-by: Michal Hocko <mhocko@suse.com>
>> ---
>> Hi,
>> Andrew's sent the MAP_FIXED_NOREPLACE to Linus for the upcoming merge
>> window. So here we go with the man page update.
>>
>>  man2/mmap.2 | 27 +++++++++++++++++++++++++++
>>  1 file changed, 27 insertions(+)
>>
>> diff --git a/man2/mmap.2 b/man2/mmap.2
>> index ea64eb8f0dcc..f702f3e4eba2 100644
>> --- a/man2/mmap.2
>> +++ b/man2/mmap.2
>> @@ -261,6 +261,27 @@ Examples include
>>  and the PAM libraries
>>  .UR http://www.linux-pam.org
>>  .UE .
>> +Newer kernels
>> +(Linux 4.17 and later) have a
>> +.B MAP_FIXED_NOREPLACE
>> +option that avoids the corruption problem; if available, MAP_FIXED_NOREPLACE
>> +should be preferred over MAP_FIXED.
> 
> This still looks wrong to me. There are legitimate uses for MAP_FIXED,
> and for most users of MAP_FIXED that I'm aware of, MAP_FIXED_NOREPLACE
> wouldn't work while MAP_FIXED works perfectly well.
> 
> MAP_FIXED is for when you have already reserved the targeted memory
> area using another VMA; MAP_FIXED_NOREPLACE is for when you haven't.

That's a nice summary, I hope it shows up in your upcoming patch. I recall
that we went back and forth, trying to find a balance of explaining
this feature, without providing overly-elaborate examples (which I tend
toward).

> Please don't make it sound as if MAP_FIXED is always wrong.
> 

Agreed.

thanks,
-- 
John Hubbard
NVIDIA

^ permalink raw reply

* Re: [PATCH 24/24] debugfs: Restrict debugfs when the kernel is locked down
From: Greg KH @ 2018-04-11 20:33 UTC (permalink / raw)
  To: David Howells
  Cc: torvalds, linux-man, linux-api, jmorris, linux-kernel,
	linux-security-module
In-Reply-To: <12769.1523477356@warthog.procyon.org.uk>

On Wed, Apr 11, 2018 at 09:09:16PM +0100, David Howells wrote:
> Greg KH <greg@kroah.com> wrote:
> 
> > Why not just disable debugfs entirely?  This half-hearted way to sorta
> > lock it down is odd, it is meant to not be there at all, nothing in your
> > normal system should ever depend on it.
> > 
> > So again just don't allow it to be mounted at all, much simpler and more
> > obvious as to what is going on.
> 
> Yeah, I agree - and then I got complaints because it seems that it's been
> abused to allow drivers and userspace components to communicate.

With in-kernel code?  Please let me know and I'll go fix it up to not
allow that, as that is not ok.

I do know of some bad examples of out-of-tree code abusing debugfs to do
crazy things (battery level monitoring?), but that's their own fault...

debugfs is for DEBUGGING!  For anything you all feel should be "secure",
then just disable it entirely.

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH 06/24] kexec_load: Disable at runtime if the kernel is locked down
From: Mimi Zohar @ 2018-04-11 20:09 UTC (permalink / raw)
  To: Eric W. Biederman, David Howells
  Cc: torvalds, linux-man, linux-api, jmorris, linux-kernel,
	linux-security-module
In-Reply-To: <87po35k1q6.fsf@xmission.com>

On Wed, 2018-04-11 at 14:00 -0500, Eric W. Biederman wrote:
> David Howells <dhowells@redhat.com> writes:
> 
> > From: Matthew Garrett <mjg59@srcf.ucam.org>
> >
> > The kexec_load() syscall permits the loading and execution of arbitrary
> > code in ring 0, which is something that lock-down is meant to prevent. It
> > makes sense to disable kexec_load() in this situation.
> >
> > This does not affect kexec_file_load() syscall which can check for a
> > signature on the image to be booted.
> 
> Maybing I am missing it but I am not seeing anything that would require
> kexec_file_load be configured such that it checks the loaded kernel.
> 
> Without that I don't see the point of disabling kexec_load.
> 
> Nacked-by: "Eric W. Biederman" <ebiederm@xmission.com>

The IMA "secure boot" policy requires the kexec image to be signed.
 This call to kernel_is_locked_down() could be replaced with a call
to security_kernel_read_file(NULL, READING_KEXEC_IMAGE).

It would be similar to the existing init_module syscall calling
security_kernel_read_file().

Mimi

^ permalink raw reply

* Re: [PATCH 24/24] debugfs: Restrict debugfs when the kernel is locked down
From: David Howells @ 2018-04-11 20:09 UTC (permalink / raw)
  To: Greg KH
  Cc: dhowells, torvalds, linux-man, linux-api, jmorris, linux-kernel,
	linux-security-module
In-Reply-To: <20180411195436.GA7126@kroah.com>

Greg KH <greg@kroah.com> wrote:

> Why not just disable debugfs entirely?  This half-hearted way to sorta
> lock it down is odd, it is meant to not be there at all, nothing in your
> normal system should ever depend on it.
> 
> So again just don't allow it to be mounted at all, much simpler and more
> obvious as to what is going on.

Yeah, I agree - and then I got complaints because it seems that it's been
abused to allow drivers and userspace components to communicate.

David

^ permalink raw reply

* Re: [PATCH 24/24] debugfs: Restrict debugfs when the kernel is locked down
From: David Howells @ 2018-04-11 20:08 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: dhowells, torvalds, linux-man, linux-api, jmorris, linux-kernel,
	linux-security-module
In-Reply-To: <87zi29k26z.fsf@xmission.com>

Eric W. Biederman <ebiederm@xmission.com> wrote:

> Why is mounting debugfs allowed at all?  Last I checked (it has been a while)
> the code quality of debugfs was fine for debugging but debugfs was not
> safe to mount on a production system.
> 
> Maybe the code quality is better now but for a filesystem that is
> not supposed to be needed for developers letting us mount debugfs
> seems odd.

I agree.  But debugfs has been abused and it seems that there are some things
that use it as an interface between a kernel driver and the userspace side.

David

^ permalink raw reply

* Re: [PATCH 06/24] kexec_load: Disable at runtime if the kernel is locked down
From: David Howells @ 2018-04-11 20:05 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: dhowells, torvalds, linux-man, linux-api, jmorris, linux-kernel,
	linux-security-module
In-Reply-To: <87po35k1q6.fsf@xmission.com>

Eric W. Biederman <ebiederm@xmission.com> wrote:

> Maybing I am missing it but I am not seeing anything that would require
> kexec_file_load be configured such that it checks the loaded kernel.
> 
> Without that I don't see the point of disabling kexec_load.

I meant to remove this patch too.

David

^ permalink raw reply

* Re: [PATCH 01/24] Add the ability to lock down access to the running kernel image
From: Greg KH @ 2018-04-11 19:56 UTC (permalink / raw)
  To: Miguel Ojeda
  Cc: Randy Dunlap, David Howells, Linus Torvalds, linux-man, linux-api,
	jmorris, linux-kernel, linux-security-module
In-Reply-To: <CANiq72n=b3ghgHzknhY=fNP9=f8fi8OuW7OPQHXQGJH_aWrT-w@mail.gmail.com>

On Wed, Apr 11, 2018 at 08:50:00PM +0200, Miguel Ojeda wrote:
> On Wed, Apr 11, 2018 at 7:37 PM, Randy Dunlap <rdunlap@infradead.org> wrote:
> > On 04/11/2018 09:24 AM, David Howells wrote:
> >
> >> diff --git a/security/lock_down.c b/security/lock_down.c
> >> new file mode 100644
> >> index 000000000000..f35ffdd096ad
> >> --- /dev/null
> >> +++ b/security/lock_down.c
> >> @@ -0,0 +1,65 @@
> >> +/* Lock down the kernel
> >> + *
> >> + * Copyright (C) 2016 Red Hat, Inc. All Rights Reserved.
> >> + * Written by David Howells (dhowells@redhat.com)
> >> + *
> >> + * This program is free software; you can redistribute it and/or
> >> + * modify it under the terms of the GNU General Public Licence
> >> + * as published by the Free Software Foundation; either version
> >> + * 2 of the Licence, or (at your option) any later version.
> >
> > fsf.org spells that Licence word as License. :)
> 
> Given this is a new file, simply use the SPF identifier on top.

checkpatch.pl should now check for this as well.

^ permalink raw reply

* Re: [PATCH 24/24] debugfs: Restrict debugfs when the kernel is locked down
From: Greg KH @ 2018-04-11 19:54 UTC (permalink / raw)
  To: David Howells
  Cc: torvalds, linux-man, linux-api, jmorris, linux-kernel,
	linux-security-module
In-Reply-To: <152346403637.4030.15247096217928429102.stgit@warthog.procyon.org.uk>

On Wed, Apr 11, 2018 at 05:27:16PM +0100, David Howells wrote:
> Disallow opening of debugfs files that might be used to muck around when
> the kernel is locked down as various drivers give raw access to hardware
> through debugfs.  Given the effort of auditing all 2000 or so files and
> manually fixing each one as necessary, I've chosen to apply a heuristic
> instead.  The following changes are made:
> 
>  (1) chmod and chown are disallowed on debugfs objects (though the root dir
>      can be modified by mount and remount, but I'm not worried about that).
> 
>  (2) When the kernel is locked down, only files with the following criteria
>      are permitted to be opened:
> 
> 	- The file must have mode 00444
> 	- The file must not have ioctl methods
> 	- The file must not have mmap
> 
>  (3) When the kernel is locked down, files may only be opened for reading.
> 
> Normal device interaction should be done through configfs, sysfs or a
> miscdev, not debugfs.
> 
> Note that this makes it unnecessary to specifically lock down show_dsts(),
> show_devs() and show_call() in the asus-wmi driver.
> 
> I would actually prefer to lock down all files by default and have the
> the files unlocked by the creator.  This is tricky to manage correctly,
> though, as there are 19 creation functions and ~1600 call sites (some of
> them in loops scanning tables).

Why not just disable debugfs entirely?  This half-hearted way to sorta
lock it down is odd, it is meant to not be there at all, nothing in your
normal system should ever depend on it.

So again just don't allow it to be mounted at all, much simpler and more
obvious as to what is going on.

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH 06/24] kexec_load: Disable at runtime if the kernel is locked down
From: Eric W. Biederman @ 2018-04-11 19:00 UTC (permalink / raw)
  To: David Howells
  Cc: torvalds, linux-man, linux-api, jmorris, linux-kernel,
	linux-security-module
In-Reply-To: <152346391877.4030.6270466586590461223.stgit@warthog.procyon.org.uk>

David Howells <dhowells@redhat.com> writes:

> From: Matthew Garrett <mjg59@srcf.ucam.org>
>
> The kexec_load() syscall permits the loading and execution of arbitrary
> code in ring 0, which is something that lock-down is meant to prevent. It
> makes sense to disable kexec_load() in this situation.
>
> This does not affect kexec_file_load() syscall which can check for a
> signature on the image to be booted.

Maybing I am missing it but I am not seeing anything that would require
kexec_file_load be configured such that it checks the loaded kernel.

Without that I don't see the point of disabling kexec_load.

Nacked-by: "Eric W. Biederman" <ebiederm@xmission.com>


Eric




> Signed-off-by: Matthew Garrett <mjg59@srcf.ucam.org>
> Signed-off-by: David Howells <dhowells@redhat.com>
> Acked-by: Dave Young <dyoung@redhat.com>
> Reviewed-by: "Lee, Chun-Yi" <jlee@suse.com>
> Reviewed-by: James Morris <james.l.morris@oracle.com>
> cc: kexec@lists.infradead.org
> ---
>
>  kernel/kexec.c |    7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/kernel/kexec.c b/kernel/kexec.c
> index aed8fb2564b3..1553ac765e73 100644
> --- a/kernel/kexec.c
> +++ b/kernel/kexec.c
> @@ -199,6 +199,13 @@ static inline int kexec_load_check(unsigned long nr_segments,
>  	if (!capable(CAP_SYS_BOOT) || kexec_load_disabled)
>  		return -EPERM;
>  
> +	/*
> +	 * kexec can be used to circumvent module loading restrictions, so
> +	 * prevent loading in that case
> +	 */
> +	if (kernel_is_locked_down("kexec of unsigned images"))
> +		return -EPERM;
> +
>  	/*
>  	 * Verify we have a legal set of flags
>  	 * This leaves us room for future extensions.

^ permalink raw reply

* Re: [PATCH 24/24] debugfs: Restrict debugfs when the kernel is locked down
From: Eric W. Biederman @ 2018-04-11 18:50 UTC (permalink / raw)
  To: David Howells
  Cc: torvalds, linux-man, linux-api, jmorris, linux-kernel,
	linux-security-module
In-Reply-To: <152346403637.4030.15247096217928429102.stgit@warthog.procyon.org.uk>

David Howells <dhowells@redhat.com> writes:

> Disallow opening of debugfs files that might be used to muck around when
> the kernel is locked down as various drivers give raw access to hardware
> through debugfs.  Given the effort of auditing all 2000 or so files and
> manually fixing each one as necessary, I've chosen to apply a heuristic
> instead.  The following changes are made:
>
>  (1) chmod and chown are disallowed on debugfs objects (though the root dir
>      can be modified by mount and remount, but I'm not worried about that).
>
>  (2) When the kernel is locked down, only files with the following criteria
>      are permitted to be opened:
>
> 	- The file must have mode 00444
> 	- The file must not have ioctl methods
> 	- The file must not have mmap
>
>  (3) When the kernel is locked down, files may only be opened for reading.
>
> Normal device interaction should be done through configfs, sysfs or a
> miscdev, not debugfs.

> Note that this makes it unnecessary to specifically lock down show_dsts(),
> show_devs() and show_call() in the asus-wmi driver.
>
> I would actually prefer to lock down all files by default and have the
> the files unlocked by the creator.  This is tricky to manage correctly,
> though, as there are 19 creation functions and ~1600 call sites (some of
> them in loops scanning tables).

Why is mounting debugfs allowed at all?  Last I checked (it has been a while)
the code quality of debugfs was fine for debugging but debugfs was not
safe to mount on a production system.

Maybe the code quality is better now but for a filesystem that is
not supposed to be needed for developers letting us mount debugfs
seems odd.

Eric

^ permalink raw reply

* Re: [PATCH 01/24] Add the ability to lock down access to the running kernel image
From: Miguel Ojeda @ 2018-04-11 18:50 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: David Howells, Linus Torvalds, linux-man, linux-api, jmorris,
	linux-kernel, linux-security-module
In-Reply-To: <6a37b428-d9fb-12d5-8d36-8a032984af8c@infradead.org>

On Wed, Apr 11, 2018 at 7:37 PM, Randy Dunlap <rdunlap@infradead.org> wrote:
> On 04/11/2018 09:24 AM, David Howells wrote:
>
>> diff --git a/security/lock_down.c b/security/lock_down.c
>> new file mode 100644
>> index 000000000000..f35ffdd096ad
>> --- /dev/null
>> +++ b/security/lock_down.c
>> @@ -0,0 +1,65 @@
>> +/* Lock down the kernel
>> + *
>> + * Copyright (C) 2016 Red Hat, Inc. All Rights Reserved.
>> + * Written by David Howells (dhowells@redhat.com)
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public Licence
>> + * as published by the Free Software Foundation; either version
>> + * 2 of the Licence, or (at your option) any later version.
>
> fsf.org spells that Licence word as License. :)

Given this is a new file, simply use the SPF identifier on top.

Cheers,
Miguel

^ permalink raw reply

* Re: [PATCH 01/24] Add the ability to lock down access to the running kernel image
From: Justin Forbes @ 2018-04-11 18:35 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: David Howells, linux-man, Linux API, James Morris,
	Linux Kernel Mailing List, LSM List
In-Reply-To: <CA+55aFzPo-dip8dgyo0U+g5qai9SAJU+D1c+AFJ0zV9_PBAB8Q@mail.gmail.com>

On Wed, Apr 11, 2018 at 1:09 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Wed, Apr 11, 2018 at 9:24 AM, David Howells <dhowells@redhat.com> wrote:
>> Provide a single call to allow kernel code to determine whether the system
>> should be locked down, thereby disallowing various accesses that might
>> allow the running kernel image to be changed, including:
>>
>>  - /dev/mem and similar
>>  - Loading of unauthorised modules
>>  - Fiddling with MSR registers
>>  - Suspend to disk managed by the kernel
>>  - Use of device DMA
>
> So what I stlll absolutely detest about  this series is that I think
> many of these things should simply be done as separate config options.
>
> For example, if the distro is sure that it doesn't need /dev/mem, then
> why the hell is  this tied to "lockdown" that then may have to be
> disabled because *other* changes may not be acceptable (eg people may
> need that device DMA, or whatever).
>
> If that /dev/mem access prevention was just instead done as an even
> stricter mode of the existing CONFIG_STRICT_DEVMEM, it could just be
> enabled unconditionally.
>
> So none of these patches raise my hackles per se. But what continues
> to makes me very very uncomfortable is how this is all tied together.
>
> Why is this one magical mode that then - because it has such a big
> impact - has to be enabled/disabled as a single magical mode and with
> very odd rules?
>
> I think a lot of people would be happier if this wasn't so incestuous
> and mixing together independent things under one name, and one flag.
>
> I think a lot of the secure boot problems were exacerbated by that mixup.
>
> So I would seriously ask that the distros that have been using these
> patches look at which parts of lockdown they could make unconditional
> (because it doesn't break machines), and which ones need that escape
> clause.
>

Optionally, it might make sense to add separate config options for
each of these pieces which can be unconditionally enabled, and a
separate option for secure boot which selects all of them? As much as
I hate select, it might make sense here.  Of course the flip side to
that, is users no longer have one big switch "turn off secure boot"
which turns it all off in case of trouble.

Justin

^ permalink raw reply

* Re: [PATCH 01/24] Add the ability to lock down access to the running kernel image
From: Linus Torvalds @ 2018-04-11 18:09 UTC (permalink / raw)
  To: David Howells
  Cc: linux-man, Linux API, James Morris, Linux Kernel Mailing List,
	LSM List
In-Reply-To: <152346388583.4030.15146667041427303547.stgit@warthog.procyon.org.uk>

On Wed, Apr 11, 2018 at 9:24 AM, David Howells <dhowells@redhat.com> wrote:
> Provide a single call to allow kernel code to determine whether the system
> should be locked down, thereby disallowing various accesses that might
> allow the running kernel image to be changed, including:
>
>  - /dev/mem and similar
>  - Loading of unauthorised modules
>  - Fiddling with MSR registers
>  - Suspend to disk managed by the kernel
>  - Use of device DMA

So what I stlll absolutely detest about  this series is that I think
many of these things should simply be done as separate config options.

For example, if the distro is sure that it doesn't need /dev/mem, then
why the hell is  this tied to "lockdown" that then may have to be
disabled because *other* changes may not be acceptable (eg people may
need that device DMA, or whatever).

If that /dev/mem access prevention was just instead done as an even
stricter mode of the existing CONFIG_STRICT_DEVMEM, it could just be
enabled unconditionally.

So none of these patches raise my hackles per se. But what continues
to makes me very very uncomfortable is how this is all tied together.

Why is this one magical mode that then - because it has such a big
impact - has to be enabled/disabled as a single magical mode and with
very odd rules?

I think a lot of people would be happier if this wasn't so incestuous
and mixing together independent things under one name, and one flag.

I think a lot of the secure boot problems were exacerbated by that mixup.

So I would seriously ask that the distros that have been using these
patches look at which parts of lockdown they could make unconditional
(because it doesn't break machines), and which ones need that escape
clause.

                 Linus

^ permalink raw reply

* Re: [PATCH 01/24] Add the ability to lock down access to the running kernel image
From: David Howells @ 2018-04-11 17:49 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: dhowells, torvalds, linux-man, linux-api, jmorris, linux-kernel,
	linux-security-module
In-Reply-To: <6a37b428-d9fb-12d5-8d36-8a032984af8c@infradead.org>

Randy Dunlap <rdunlap@infradead.org> wrote:

> > +	  drivers from functioning because allowing manual configuration of
> > +	  hardware parameters is forbidden, lest a device be used to access the
> > +	  kernel by DMA.  This mostly applies to ISA devices.
> 
> 	Is DMA from non-ISA devices OK, or did I miss seeing that patch?

The issue is where you can directly command a device to do DMA, possibly by
using the wrong driver.  Ask Alan Cox.

David

^ permalink raw reply

* Re: [PATCH 01/24] Add the ability to lock down access to the running kernel image
From: Randy Dunlap @ 2018-04-11 17:37 UTC (permalink / raw)
  To: David Howells, torvalds
  Cc: linux-man, linux-api, jmorris, linux-kernel,
	linux-security-module
In-Reply-To: <152346388583.4030.15146667041427303547.stgit@warthog.procyon.org.uk>

On 04/11/2018 09:24 AM, David Howells wrote:
> ---
> 
>  arch/x86/kernel/setup.c |    2 +
>  include/linux/kernel.h  |   32 +++++++++++++++++++++++
>  security/Kconfig        |   23 ++++++++++++++++-
>  security/Makefile       |    3 ++
>  security/lock_down.c    |   65 +++++++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 124 insertions(+), 1 deletion(-)
>  create mode 100644 security/lock_down.c

> diff --git a/security/Kconfig b/security/Kconfig
> index c4302067a3ad..a68e5bdebad5 100644
> --- a/security/Kconfig
> +++ b/security/Kconfig
> @@ -231,6 +231,28 @@ config STATIC_USERMODEHELPER_PATH
>  	  If you wish for all usermode helper programs to be disabled,
>  	  specify an empty string here (i.e. "").
>  
> +config LOCK_DOWN_KERNEL
> +	bool "Allow the kernel to be 'locked down'"
> +	help
> +	  Allow the kernel to be locked down.  Locking down the kernel turns
> +	  off various features that might otherwise allow access to the kernel

	  s/turns off/disables/

> +	  image (eg. setting MSR registers).

	         e.g.

> +
> +	  Note, however, that locking down your kernel will prevent some

	                                   the kernel
	                                   a kernel

> +	  drivers from functioning because allowing manual configuration of
> +	  hardware parameters is forbidden, lest a device be used to access the
> +	  kernel by DMA.  This mostly applies to ISA devices.

	Is DMA from non-ISA devices OK, or did I miss seeing that patch?

> +	  The kernel lockdown can be triggered by adding lockdown=1 to the
> +	  kernel command line.


> diff --git a/security/lock_down.c b/security/lock_down.c
> new file mode 100644
> index 000000000000..f35ffdd096ad
> --- /dev/null
> +++ b/security/lock_down.c
> @@ -0,0 +1,65 @@
> +/* Lock down the kernel
> + *
> + * Copyright (C) 2016 Red Hat, Inc. All Rights Reserved.
> + * Written by David Howells (dhowells@redhat.com)
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public Licence
> + * as published by the Free Software Foundation; either version
> + * 2 of the Licence, or (at your option) any later version.

fsf.org spells that Licence word as License. :)

> + */


-- 
~Randy

^ permalink raw reply

* Re: [PATCH 24/24] debugfs: Restrict debugfs when the kernel is locked down
From: Randy Dunlap @ 2018-04-11 17:26 UTC (permalink / raw)
  To: David Howells, torvalds
  Cc: linux-man, linux-api, jmorris, linux-kernel,
	linux-security-module
In-Reply-To: <152346403637.4030.15247096217928429102.stgit@warthog.procyon.org.uk>

On 04/11/2018 09:27 AM, David Howells wrote:

> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: Andy Shevchenko <andy.shevchenko@gmail.com>
> cc: acpi4asus-user@lists.sourceforge.net
> cc: platform-driver-x86@vger.kernel.org
> cc: Matthew Garrett <mjg59@srcf.ucam.org>
> cc: Thomas Gleixner <tglx@linutronix.de>
> ---
meta-comment:

I have been dinged for not spelling "cc:" as "Cc:". I really think that
either way should be acceptable.

-- 
~Randy

^ permalink raw reply

* Re: [PATCH 18/24] Lock down module params that specify hardware parameters (eg. ioport)
From: Randy Dunlap @ 2018-04-11 17:22 UTC (permalink / raw)
  To: David Howells, torvalds
  Cc: linux-man, linux-api, jmorris, linux-kernel,
	linux-security-module
In-Reply-To: <152346399627.4030.8043878470312297727.stgit@warthog.procyon.org.uk>

On 04/11/2018 09:26 AM, David Howells wrote:
> Provided an annotation for module parameters that specify hardware
> parameters (such as io ports, iomem addresses, irqs, dma channels, fixed
> dma buffers and other types).
> 
> Suggested-by: Alan Cox <gnomes@lxorguk.ukuu.org.uk>
> Signed-off-by: David Howells <dhowells@redhat.com>
> ---
> 
>  kernel/params.c |   26 +++++++++++++++++++++-----
>  1 file changed, 21 insertions(+), 5 deletions(-)
> 
> diff --git a/kernel/params.c b/kernel/params.c
> index cc9108c2a1fd..2c08c4aa376b 100644
> --- a/kernel/params.c
> +++ b/kernel/params.c
> @@ -108,13 +108,19 @@ bool parameq(const char *a, const char *b)
>  	return parameqn(a, b, strlen(a)+1);
>  }
>  
> -static void param_check_unsafe(const struct kernel_param *kp)
> +static bool param_check_unsafe(const struct kernel_param *kp,
> +			       const char *doing)
>  {
>  	if (kp->flags & KERNEL_PARAM_FL_UNSAFE) {
>  		pr_warn("Setting dangerous option %s - tainting kernel\n",
>  			kp->name);
>  		add_taint(TAINT_USER, LOCKDEP_STILL_OK);
>  	}
> +
> +	if (kp->flags & KERNEL_PARAM_FL_HWPARAM &&
> +	    kernel_is_locked_down("Command line-specified device addresses, irqs and dma channels"))

s/dma/DMA/

Other patches use PCI instead of pci, CIS instead of cis, MMIO instead of mmio,
BPF instead of bpf, MSR instead of msr, etc...


> +		return false;
> +	return true;
>  }
>  
>  static int parse_one(char *param,


-- 
~Randy

^ permalink raw reply

* Re: [PATCH] mmap.2: document new MAP_FIXED_NOREPLACE flag
From: Michal Hocko @ 2018-04-11 17:06 UTC (permalink / raw)
  To: Jann Horn
  Cc: Michael Kerrisk, John Hubbard, Andrew Morton, Linux-MM, LKML,
	Linux API
In-Reply-To: <CAG48ez2wYqxJEHgZCz5g6ZYBY4_qDcYWSGAErC8pUzmrW62rug@mail.gmail.com>

On Wed 11-04-18 18:40:09, Jann Horn wrote:
> On Wed, Apr 11, 2018 at 6:36 PM, Michal Hocko <mhocko@kernel.org> wrote:
> > On Wed 11-04-18 17:37:46, Jann Horn wrote:
> >> On Wed, Apr 11, 2018 at 2:04 PM,  <mhocko@kernel.org> wrote:
> >> > From: Michal Hocko <mhocko@suse.com>
> >> >
> >> > 4.17+ kernels offer a new MAP_FIXED_NOREPLACE flag which allows the caller to
> >> > atomicaly probe for a given address range.
> >> >
> >> > [wording heavily updated by John Hubbard <jhubbard@nvidia.com>]
> >> > Signed-off-by: Michal Hocko <mhocko@suse.com>
> >> > ---
> >> > Hi,
> >> > Andrew's sent the MAP_FIXED_NOREPLACE to Linus for the upcoming merge
> >> > window. So here we go with the man page update.
> >> >
> >> >  man2/mmap.2 | 27 +++++++++++++++++++++++++++
> >> >  1 file changed, 27 insertions(+)
> >> >
> >> > diff --git a/man2/mmap.2 b/man2/mmap.2
> >> > index ea64eb8f0dcc..f702f3e4eba2 100644
> >> > --- a/man2/mmap.2
> >> > +++ b/man2/mmap.2
> >> > @@ -261,6 +261,27 @@ Examples include
> >> >  and the PAM libraries
> >> >  .UR http://www.linux-pam.org
> >> >  .UE .
> >> > +Newer kernels
> >> > +(Linux 4.17 and later) have a
> >> > +.B MAP_FIXED_NOREPLACE
> >> > +option that avoids the corruption problem; if available, MAP_FIXED_NOREPLACE
> >> > +should be preferred over MAP_FIXED.
> >>
> >> This still looks wrong to me. There are legitimate uses for MAP_FIXED,
> >> and for most users of MAP_FIXED that I'm aware of, MAP_FIXED_NOREPLACE
> >> wouldn't work while MAP_FIXED works perfectly well.
> >>
> >> MAP_FIXED is for when you have already reserved the targeted memory
> >> area using another VMA; MAP_FIXED_NOREPLACE is for when you haven't.
> >> Please don't make it sound as if MAP_FIXED is always wrong.
> >
> > Well, this was suggested by John. I think, nobody is objecting that
> > MAP_FIXED has legitimate usecases. The above text just follows up on
> > the previous section which emphasises the potential memory corruption
> > problems and it suggests that a new flag is safe with that regards.
> >
> > If you have specific wording that would be better I am open for changes.
> 
> I guess I'd probably also want to change the previous text; so I
> should probably send a followup patch once this one has landed.

yeah, that sounds like a better plan.
-- 
Michal Hocko
SUSE Labs

^ permalink raw reply

* Re: [PATCH 02/24] Add a SysRq option to lift kernel lockdown
From: Jann Horn @ 2018-04-11 17:05 UTC (permalink / raw)
  To: David Howells
  Cc: Linus Torvalds, linux-man, Linux API, James Morris, kernel list,
	linux-security-module
In-Reply-To: <152346389240.4030.11187964053014260180.stgit@warthog.procyon.org.uk>

On Wed, Apr 11, 2018 at 6:24 PM, David Howells <dhowells@redhat.com> wrote:
> From: Kyle McMartin <kyle@redhat.com>
>
> Make an option to provide a sysrq key that will lift the kernel lockdown,
> thereby allowing the running kernel image to be accessed and modified.
>
> On x86 this is triggered with SysRq+x, but this key may not be available on
> all arches, so it is set by setting LOCKDOWN_LIFT_KEY in asm/setup.h.
> Since this macro must be defined in an arch to be able to use this facility
> for that arch, the Kconfig option is restricted to arches that support it.

In the current form, this is probably incompatible with USB/IP (which
Debian seems to be shipping as a module by default), right? And
perhaps also with dummy_hcd (if I understand correctly what it's
doing)?

^ permalink raw reply

* Re: [PATCH 01/24] Add the ability to lock down access to the running kernel image
From: Jann Horn @ 2018-04-11 16:44 UTC (permalink / raw)
  To: David Howells
  Cc: Linus Torvalds, linux-man, Linux API, James Morris, kernel list,
	linux-security-module
In-Reply-To: <152346388583.4030.15146667041427303547.stgit@warthog.procyon.org.uk>

On Wed, Apr 11, 2018 at 6:24 PM, David Howells <dhowells@redhat.com> wrote:
> Provide a single call to allow kernel code to determine whether the system
> should be locked down, thereby disallowing various accesses that might
> allow the running kernel image to be changed, including:
>
>  - /dev/mem and similar
>  - Loading of unauthorised modules
>  - Fiddling with MSR registers
>  - Suspend to disk managed by the kernel
>  - Use of device DMA
>
> Two kernel configuration options are provided:
>
>  (*) CONFIG_LOCK_DOWN_KERNEL
>
>      This makes lockdown available and applies it to all the points that
>      need to be locked down if the mode is set.  Lockdown mode can be
>      enabled by providing:
>
>         lockdown=1
>
>      on the command line.
>
>  (*) CONFIG_LOCK_DOWN_MANDATORY
>
>      This forces lockdown on at compile time, overriding the command line
>      option.
>
> init_lockdown() is used as a hook from which lockdown can be managed in
> future.  It has to be called from arch setup code before things like ACPI
> are enabled.
>
> Note that, with the other changes in this series, if lockdown mode is
> enabled, the kernel will not be able to use certain drivers as the ability
> to manually configure hardware parameters would then be prohibited.  This
> primarily applies to ISA hardware devices.
>
> Signed-off-by: David Howells <dhowells@redhat.com>
> ---
[...]
> diff --git a/security/lock_down.c b/security/lock_down.c
> new file mode 100644
> index 000000000000..f35ffdd096ad
> --- /dev/null
> +++ b/security/lock_down.c
[...]
> +/*
> + * Lock the kernel down from very early in the arch setup.  This must happen
> + * prior to things like ACPI being initialised.
> + */

Pedantic nit: I think this comment is wrong now? This function
actually just prints stuff.

> +void __init init_lockdown(void)
> +{
> +#ifdef CONFIG_LOCK_DOWN_MANDATORY
> +       pr_notice("Kernel is locked down from config; see man kernel_lockdown.7\n");
> +#endif
> +}

^ permalink raw reply

* Re: [PATCH] mmap.2: document new MAP_FIXED_NOREPLACE flag
From: Jann Horn @ 2018-04-11 16:40 UTC (permalink / raw)
  To: Michal Hocko
  Cc: Michael Kerrisk, John Hubbard, Andrew Morton, Linux-MM, LKML,
	Linux API
In-Reply-To: <20180411163631.GL23400@dhcp22.suse.cz>

On Wed, Apr 11, 2018 at 6:36 PM, Michal Hocko <mhocko@kernel.org> wrote:
> On Wed 11-04-18 17:37:46, Jann Horn wrote:
>> On Wed, Apr 11, 2018 at 2:04 PM,  <mhocko@kernel.org> wrote:
>> > From: Michal Hocko <mhocko@suse.com>
>> >
>> > 4.17+ kernels offer a new MAP_FIXED_NOREPLACE flag which allows the caller to
>> > atomicaly probe for a given address range.
>> >
>> > [wording heavily updated by John Hubbard <jhubbard@nvidia.com>]
>> > Signed-off-by: Michal Hocko <mhocko@suse.com>
>> > ---
>> > Hi,
>> > Andrew's sent the MAP_FIXED_NOREPLACE to Linus for the upcoming merge
>> > window. So here we go with the man page update.
>> >
>> >  man2/mmap.2 | 27 +++++++++++++++++++++++++++
>> >  1 file changed, 27 insertions(+)
>> >
>> > diff --git a/man2/mmap.2 b/man2/mmap.2
>> > index ea64eb8f0dcc..f702f3e4eba2 100644
>> > --- a/man2/mmap.2
>> > +++ b/man2/mmap.2
>> > @@ -261,6 +261,27 @@ Examples include
>> >  and the PAM libraries
>> >  .UR http://www.linux-pam.org
>> >  .UE .
>> > +Newer kernels
>> > +(Linux 4.17 and later) have a
>> > +.B MAP_FIXED_NOREPLACE
>> > +option that avoids the corruption problem; if available, MAP_FIXED_NOREPLACE
>> > +should be preferred over MAP_FIXED.
>>
>> This still looks wrong to me. There are legitimate uses for MAP_FIXED,
>> and for most users of MAP_FIXED that I'm aware of, MAP_FIXED_NOREPLACE
>> wouldn't work while MAP_FIXED works perfectly well.
>>
>> MAP_FIXED is for when you have already reserved the targeted memory
>> area using another VMA; MAP_FIXED_NOREPLACE is for when you haven't.
>> Please don't make it sound as if MAP_FIXED is always wrong.
>
> Well, this was suggested by John. I think, nobody is objecting that
> MAP_FIXED has legitimate usecases. The above text just follows up on
> the previous section which emphasises the potential memory corruption
> problems and it suggests that a new flag is safe with that regards.
>
> If you have specific wording that would be better I am open for changes.

I guess I'd probably also want to change the previous text; so I
should probably send a followup patch once this one has landed.

>> > +.TP
>> > +.BR MAP_FIXED_NOREPLACE " (since Linux 4.17)"
>> > +Similar to MAP_FIXED with respect to the
>> > +.I
>> > +addr
>> > +enforcement, but different in that MAP_FIXED_NOREPLACE never clobbers a pre-existing
>> > +mapped range. If the requested range would collide with an existing
>> > +mapping, then this call fails with
>> > +.B EEXIST.
>> > +This flag can therefore be used as a way to atomically (with respect to other
>> > +threads) attempt to map an address range: one thread will succeed; all others
>> > +will report failure. Please note that older kernels which do not recognize this
>> > +flag will typically (upon detecting a collision with a pre-existing mapping)
>> > +fall back to a "non-MAP_FIXED" type of behavior: they will return an address that
>> > +is different than the requested one. Therefore, backward-compatible software
>> > +should check the returned address against the requested address.
>> >  .TP
>> >  .B MAP_GROWSDOWN
>> >  This flag is used for stacks.
>> > @@ -487,6 +508,12 @@ is not a valid file descriptor (and
>> >  .B MAP_ANONYMOUS
>> >  was not set).
>> >  .TP
>> > +.B EEXIST
>> > +range covered by
>> > +.IR addr ,
>> > +.IR length
>> > +is clashing with an existing mapping.
>>
>> Maybe add something like ", and MAP_FIXED_NOREPLACE was specified"? I
>> think most manpages explicitly document which error conditions can be
>> triggered by which flags.
>
> sure, no objection from me.
>
> --
> Michal Hocko
> SUSE Labs

^ permalink raw reply

* Re: [PATCH] mmap.2: document new MAP_FIXED_NOREPLACE flag
From: Michal Hocko @ 2018-04-11 16:36 UTC (permalink / raw)
  To: Jann Horn
  Cc: Michael Kerrisk, John Hubbard, Andrew Morton, Linux-MM, LKML,
	Linux API
In-Reply-To: <CAG48ez3BS5EtnrhFQUGYY9MKGOUHzFbhauJQd361uTwy2pBEeg@mail.gmail.com>

On Wed 11-04-18 17:37:46, Jann Horn wrote:
> On Wed, Apr 11, 2018 at 2:04 PM,  <mhocko@kernel.org> wrote:
> > From: Michal Hocko <mhocko@suse.com>
> >
> > 4.17+ kernels offer a new MAP_FIXED_NOREPLACE flag which allows the caller to
> > atomicaly probe for a given address range.
> >
> > [wording heavily updated by John Hubbard <jhubbard@nvidia.com>]
> > Signed-off-by: Michal Hocko <mhocko@suse.com>
> > ---
> > Hi,
> > Andrew's sent the MAP_FIXED_NOREPLACE to Linus for the upcoming merge
> > window. So here we go with the man page update.
> >
> >  man2/mmap.2 | 27 +++++++++++++++++++++++++++
> >  1 file changed, 27 insertions(+)
> >
> > diff --git a/man2/mmap.2 b/man2/mmap.2
> > index ea64eb8f0dcc..f702f3e4eba2 100644
> > --- a/man2/mmap.2
> > +++ b/man2/mmap.2
> > @@ -261,6 +261,27 @@ Examples include
> >  and the PAM libraries
> >  .UR http://www.linux-pam.org
> >  .UE .
> > +Newer kernels
> > +(Linux 4.17 and later) have a
> > +.B MAP_FIXED_NOREPLACE
> > +option that avoids the corruption problem; if available, MAP_FIXED_NOREPLACE
> > +should be preferred over MAP_FIXED.
> 
> This still looks wrong to me. There are legitimate uses for MAP_FIXED,
> and for most users of MAP_FIXED that I'm aware of, MAP_FIXED_NOREPLACE
> wouldn't work while MAP_FIXED works perfectly well.
> 
> MAP_FIXED is for when you have already reserved the targeted memory
> area using another VMA; MAP_FIXED_NOREPLACE is for when you haven't.
> Please don't make it sound as if MAP_FIXED is always wrong.

Well, this was suggested by John. I think, nobody is objecting that
MAP_FIXED has legitimate usecases. The above text just follows up on
the previous section which emphasises the potential memory corruption
problems and it suggests that a new flag is safe with that regards.

If you have specific wording that would be better I am open for changes.

> > +.TP
> > +.BR MAP_FIXED_NOREPLACE " (since Linux 4.17)"
> > +Similar to MAP_FIXED with respect to the
> > +.I
> > +addr
> > +enforcement, but different in that MAP_FIXED_NOREPLACE never clobbers a pre-existing
> > +mapped range. If the requested range would collide with an existing
> > +mapping, then this call fails with
> > +.B EEXIST.
> > +This flag can therefore be used as a way to atomically (with respect to other
> > +threads) attempt to map an address range: one thread will succeed; all others
> > +will report failure. Please note that older kernels which do not recognize this
> > +flag will typically (upon detecting a collision with a pre-existing mapping)
> > +fall back to a "non-MAP_FIXED" type of behavior: they will return an address that
> > +is different than the requested one. Therefore, backward-compatible software
> > +should check the returned address against the requested address.
> >  .TP
> >  .B MAP_GROWSDOWN
> >  This flag is used for stacks.
> > @@ -487,6 +508,12 @@ is not a valid file descriptor (and
> >  .B MAP_ANONYMOUS
> >  was not set).
> >  .TP
> > +.B EEXIST
> > +range covered by
> > +.IR addr ,
> > +.IR length
> > +is clashing with an existing mapping.
> 
> Maybe add something like ", and MAP_FIXED_NOREPLACE was specified"? I
> think most manpages explicitly document which error conditions can be
> triggered by which flags.

sure, no objection from me.

-- 
Michal Hocko
SUSE Labs

^ permalink raw reply

* [PATCH 24/24] debugfs: Restrict debugfs when the kernel is locked down
From: David Howells @ 2018-04-11 16:27 UTC (permalink / raw)
  To: torvalds
  Cc: linux-man, linux-api, jmorris, linux-kernel, dhowells,
	linux-security-module
In-Reply-To: <152346387861.4030.4408662483445703127.stgit@warthog.procyon.org.uk>

Disallow opening of debugfs files that might be used to muck around when
the kernel is locked down as various drivers give raw access to hardware
through debugfs.  Given the effort of auditing all 2000 or so files and
manually fixing each one as necessary, I've chosen to apply a heuristic
instead.  The following changes are made:

 (1) chmod and chown are disallowed on debugfs objects (though the root dir
     can be modified by mount and remount, but I'm not worried about that).

 (2) When the kernel is locked down, only files with the following criteria
     are permitted to be opened:

	- The file must have mode 00444
	- The file must not have ioctl methods
	- The file must not have mmap

 (3) When the kernel is locked down, files may only be opened for reading.

Normal device interaction should be done through configfs, sysfs or a
miscdev, not debugfs.

Note that this makes it unnecessary to specifically lock down show_dsts(),
show_devs() and show_call() in the asus-wmi driver.

I would actually prefer to lock down all files by default and have the
the files unlocked by the creator.  This is tricky to manage correctly,
though, as there are 19 creation functions and ~1600 call sites (some of
them in loops scanning tables).

Signed-off-by: David Howells <dhowells@redhat.com>
cc: Andy Shevchenko <andy.shevchenko@gmail.com>
cc: acpi4asus-user@lists.sourceforge.net
cc: platform-driver-x86@vger.kernel.org
cc: Matthew Garrett <mjg59@srcf.ucam.org>
cc: Thomas Gleixner <tglx@linutronix.de>
---

 fs/debugfs/file.c  |   28 ++++++++++++++++++++++++++++
 fs/debugfs/inode.c |   30 ++++++++++++++++++++++++++++--
 2 files changed, 56 insertions(+), 2 deletions(-)

diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index 1f99678ff5d3..51cb894c21f2 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -136,6 +136,25 @@ void debugfs_file_put(struct dentry *dentry)
 }
 EXPORT_SYMBOL_GPL(debugfs_file_put);
 
+/*
+ * Only permit access to world-readable files when the kernel is locked down.
+ * We also need to exclude any file that has ways to write or alter it as root
+ * can bypass the permissions check.
+ */
+static bool debugfs_is_locked_down(struct inode *inode,
+				   struct file *filp,
+				   const struct file_operations *real_fops)
+{
+	if ((inode->i_mode & 07777) == 0444 &&
+	    !(filp->f_mode & FMODE_WRITE) &&
+	    !real_fops->unlocked_ioctl &&
+	    !real_fops->compat_ioctl &&
+	    !real_fops->mmap)
+		return false;
+
+	return kernel_is_locked_down("debugfs");
+}
+
 static int open_proxy_open(struct inode *inode, struct file *filp)
 {
 	struct dentry *dentry = F_DENTRY(filp);
@@ -147,6 +166,11 @@ static int open_proxy_open(struct inode *inode, struct file *filp)
 		return r == -EIO ? -ENOENT : r;
 
 	real_fops = debugfs_real_fops(filp);
+
+	r = -EPERM;
+	if (debugfs_is_locked_down(inode, filp, real_fops))
+		goto out;
+
 	real_fops = fops_get(real_fops);
 	if (!real_fops) {
 		/* Huh? Module did not clean up after itself at exit? */
@@ -272,6 +296,10 @@ static int full_proxy_open(struct inode *inode, struct file *filp)
 		return r == -EIO ? -ENOENT : r;
 
 	real_fops = debugfs_real_fops(filp);
+	r = -EPERM;
+	if (debugfs_is_locked_down(inode, filp, real_fops))
+		goto out;
+
 	real_fops = fops_get(real_fops);
 	if (!real_fops) {
 		/* Huh? Module did not cleanup after itself at exit? */
diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
index 13b01351dd1c..4daec17b8215 100644
--- a/fs/debugfs/inode.c
+++ b/fs/debugfs/inode.c
@@ -32,6 +32,31 @@ static struct vfsmount *debugfs_mount;
 static int debugfs_mount_count;
 static bool debugfs_registered;
 
+/*
+ * Don't allow access attributes to be changed whilst the kernel is locked down
+ * so that we can use the file mode as part of a heuristic to determine whether
+ * to lock down individual files.
+ */
+static int debugfs_setattr(struct dentry *dentry, struct iattr *ia)
+{
+	if ((ia->ia_valid & (ATTR_MODE | ATTR_UID | ATTR_GID)) &&
+	    kernel_is_locked_down("debugfs"))
+		return -EPERM;
+	return simple_setattr(dentry, ia);
+}
+
+static const struct inode_operations debugfs_file_inode_operations = {
+	.setattr	= debugfs_setattr,
+};
+static const struct inode_operations debugfs_dir_inode_operations = {
+	.lookup		= simple_lookup,
+	.setattr	= debugfs_setattr,
+};
+static const struct inode_operations debugfs_symlink_inode_operations = {
+	.get_link	= simple_get_link,
+	.setattr	= debugfs_setattr,
+};
+
 static struct inode *debugfs_get_inode(struct super_block *sb)
 {
 	struct inode *inode = new_inode(sb);
@@ -356,6 +381,7 @@ static struct dentry *__debugfs_create_file(const char *name, umode_t mode,
 	inode->i_mode = mode;
 	inode->i_private = data;
 
+	inode->i_op = &debugfs_file_inode_operations;
 	inode->i_fop = proxy_fops;
 	dentry->d_fsdata = (void *)((unsigned long)real_fops |
 				DEBUGFS_FSDATA_IS_REAL_FOPS_BIT);
@@ -513,7 +539,7 @@ struct dentry *debugfs_create_dir(const char *name, struct dentry *parent)
 		return failed_creating(dentry);
 
 	inode->i_mode = S_IFDIR | S_IRWXU | S_IRUGO | S_IXUGO;
-	inode->i_op = &simple_dir_inode_operations;
+	inode->i_op = &debugfs_dir_inode_operations;
 	inode->i_fop = &simple_dir_operations;
 
 	/* directory inodes start off with i_nlink == 2 (for "." entry) */
@@ -608,7 +634,7 @@ struct dentry *debugfs_create_symlink(const char *name, struct dentry *parent,
 		return failed_creating(dentry);
 	}
 	inode->i_mode = S_IFLNK | S_IRWXUGO;
-	inode->i_op = &simple_symlink_inode_operations;
+	inode->i_op = &debugfs_symlink_inode_operations;
 	inode->i_link = link;
 	d_instantiate(dentry, inode);
 	return end_creating(dentry);

^ permalink raw reply related

* [PATCH 23/24] Lock down perf
From: David Howells @ 2018-04-11 16:27 UTC (permalink / raw)
  To: torvalds
  Cc: linux-man, linux-api, jmorris, linux-kernel, dhowells,
	linux-security-module
In-Reply-To: <152346387861.4030.4408662483445703127.stgit@warthog.procyon.org.uk>

Disallow the use of certain perf facilities that might allow userspace to
access kernel data.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 kernel/events/core.c |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index fc1c330c6bd6..1922f2e0980a 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -10407,6 +10407,11 @@ SYSCALL_DEFINE5(perf_event_open,
 			return -EINVAL;
 	}
 
+	if ((attr.sample_type & PERF_SAMPLE_REGS_INTR) &&
+	    kernel_is_locked_down("PERF_SAMPLE_REGS_INTR"))
+		/* REGS_INTR can leak data, lockdown must prevent this */
+		return -EPERM;
+
 	/* Only privileged users can get physical addresses */
 	if ((attr.sample_type & PERF_SAMPLE_PHYS_ADDR) &&
 	    perf_paranoid_kernel() && !capable(CAP_SYS_ADMIN))

^ permalink raw reply related

* [PATCH 22/24] bpf: Restrict kernel image access functions when the kernel is locked down
From: David Howells @ 2018-04-11 16:27 UTC (permalink / raw)
  To: torvalds
  Cc: linux-man, linux-api, jmorris, linux-kernel, dhowells,
	linux-security-module
In-Reply-To: <152346387861.4030.4408662483445703127.stgit@warthog.procyon.org.uk>

There are some bpf functions can be used to read kernel memory:
bpf_probe_read, bpf_probe_write_user and bpf_trace_printk.  These allow
private keys in kernel memory (e.g. the hibernation image signing key) to
be read by an eBPF program.

Completely prohibit the use of BPF when the kernel is locked down.

Suggested-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Signed-off-by: David Howells <dhowells@redhat.com>
cc: netdev@vger.kernel.org
cc: Chun-Yi Lee <jlee@suse.com>
cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>
---

 kernel/bpf/syscall.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 0244973ee544..7457f2676c6d 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -2031,6 +2031,9 @@ SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, siz
 	if (sysctl_unprivileged_bpf_disabled && !capable(CAP_SYS_ADMIN))
 		return -EPERM;
 
+	if (kernel_is_locked_down("BPF"))
+		return -EPERM;
+
 	err = check_uarg_tail_zero(uattr, sizeof(attr), size);
 	if (err)
 		return err;

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox