* Re: [PATCH] mmap.2: MAP_FIXED is okay if the address range has been reserved
From: Jann Horn @ 2018-04-12 18:49 UTC (permalink / raw)
To: Michael Kerrisk-manpages, John Hubbard
Cc: linux-man, Michal Hocko, Andrew Morton, Linux-MM, lkml, Linux API
In-Reply-To: <CAKgNAkgcJ2kCTff0=7=D3zPFwpJt-9EM8yis6-4qDjfvvb8ukw@mail.gmail.com>
On Thu, Apr 12, 2018 at 8:37 PM, Michael Kerrisk (man-pages)
<mtk.manpages@gmail.com> wrote:
> Hi John,
>
> On 12 April 2018 at 20:33, John Hubbard <jhubbard@nvidia.com> wrote:
>> On 04/12/2018 08:39 AM, Jann Horn wrote:
>>> Clarify that MAP_FIXED is appropriate if the specified address range has
>>> been reserved using an existing mapping, but shouldn't be used otherwise.
>>>
>>> Signed-off-by: Jann Horn <jannh@google.com>
>>> ---
>>> man2/mmap.2 | 19 +++++++++++--------
>>> 1 file changed, 11 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/man2/mmap.2 b/man2/mmap.2
[...]
>>> .IP
>>> For example, suppose that thread A looks through
>>> @@ -284,13 +285,15 @@ and the PAM libraries
>>> .UR http://www.linux-pam.org
>>> .UE .
>>> .IP
>>> -Newer kernels
>>> -(Linux 4.17 and later) have a
>>> +For cases in which the specified memory region has not been reserved using an
>>> +existing mapping, newer kernels (Linux 4.17 and later) provide an option
>>> .B MAP_FIXED_NOREPLACE
>>> -option that avoids the corruption problem; if available,
>>> -.B MAP_FIXED_NOREPLACE
>>> -should be preferred over
>>> -.BR MAP_FIXED .
>>> +that should be used instead; older kernels require the caller to use
>>> +.I addr
>>> +as a hint (without
>>> +.BR MAP_FIXED )
>>
>> Here, I got lost: the sentence suddenly jumps into explaining non-MAP_FIXED
>> behavior, in the MAP_FIXED section. Maybe if you break up the sentence, and
>> possibly omit non-MAP_FIXED discussion, it will help.
>
> Hmmm -- true. That piece could be a little clearer.
How about something like this?
For cases in which MAP_FIXED can not be used because
the specified memory
region has not been reserved using an existing mapping,
newer kernels
(Linux 4.17 and later) provide an option
MAP_FIXED_NOREPLACE that
should be used instead. Older kernels require the
caller to use addr as a hint and take appropriate action if
the kernel places the new mapping at a different address.
John, Michael, what do you think?
> Jann, I've already pushed the existing patch. Do you want to add a patch on top?
^ permalink raw reply
* Re: [PATCH] mmap.2: MAP_FIXED is okay if the address range has been reserved
From: Michael Kerrisk (man-pages) @ 2018-04-12 18:37 UTC (permalink / raw)
To: John Hubbard
Cc: Jann Horn, linux-man, Michal Hocko, Andrew Morton, Linux-MM, lkml,
Linux API
In-Reply-To: <b617740b-fd07-e248-2ba0-9e99b0240594@nvidia.com>
Hi John,
On 12 April 2018 at 20:33, John Hubbard <jhubbard@nvidia.com> wrote:
> On 04/12/2018 08:39 AM, Jann Horn wrote:
>> Clarify that MAP_FIXED is appropriate if the specified address range has
>> been reserved using an existing mapping, but shouldn't be used otherwise.
>>
>> Signed-off-by: Jann Horn <jannh@google.com>
>> ---
>> man2/mmap.2 | 19 +++++++++++--------
>> 1 file changed, 11 insertions(+), 8 deletions(-)
>>
>> diff --git a/man2/mmap.2 b/man2/mmap.2
>> index bef8b4432..80c9ec285 100644
>> --- a/man2/mmap.2
>> +++ b/man2/mmap.2
>> @@ -253,8 +253,9 @@ Software that aspires to be portable should use this option with care,
>> keeping in mind that the exact layout of a process's memory mappings
>> is allowed to change significantly between kernel versions,
>> C library versions, and operating system releases.
>> -Furthermore, this option is extremely hazardous (when used on its own),
>> -because it forcibly removes preexisting mappings,
>> +This option should only be used when the specified memory region has
>> +already been reserved using another mapping; otherwise, it is extremely
>> +hazardous because it forcibly removes preexisting mappings,
>> making it easy for a multithreaded process to corrupt its own address space.
>
> Yes, that's clearer and provides more information than before.
>
>> .IP
>> For example, suppose that thread A looks through
>> @@ -284,13 +285,15 @@ and the PAM libraries
>> .UR http://www.linux-pam.org
>> .UE .
>> .IP
>> -Newer kernels
>> -(Linux 4.17 and later) have a
>> +For cases in which the specified memory region has not been reserved using an
>> +existing mapping, newer kernels (Linux 4.17 and later) provide an option
>> .B MAP_FIXED_NOREPLACE
>> -option that avoids the corruption problem; if available,
>> -.B MAP_FIXED_NOREPLACE
>> -should be preferred over
>> -.BR MAP_FIXED .
>> +that should be used instead; older kernels require the caller to use
>> +.I addr
>> +as a hint (without
>> +.BR MAP_FIXED )
>
> Here, I got lost: the sentence suddenly jumps into explaining non-MAP_FIXED
> behavior, in the MAP_FIXED section. Maybe if you break up the sentence, and
> possibly omit non-MAP_FIXED discussion, it will help.
Hmmm -- true. That piece could be a little clearer.
Jann, I've already pushed the existing patch. Do you want to add a patch on top?
Thanks,
Michael
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
^ permalink raw reply
* Re: [PATCH] mmap.2: MAP_FIXED is okay if the address range has been reserved
From: John Hubbard @ 2018-04-12 18:33 UTC (permalink / raw)
To: Jann Horn, mtk.manpages, linux-man, mhocko, akpm, linux-mm,
linux-kernel, linux-api
In-Reply-To: <20180412153941.170849-1-jannh@google.com>
On 04/12/2018 08:39 AM, Jann Horn wrote:
> Clarify that MAP_FIXED is appropriate if the specified address range has
> been reserved using an existing mapping, but shouldn't be used otherwise.
>
> Signed-off-by: Jann Horn <jannh@google.com>
> ---
> man2/mmap.2 | 19 +++++++++++--------
> 1 file changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/man2/mmap.2 b/man2/mmap.2
> index bef8b4432..80c9ec285 100644
> --- a/man2/mmap.2
> +++ b/man2/mmap.2
> @@ -253,8 +253,9 @@ Software that aspires to be portable should use this option with care,
> keeping in mind that the exact layout of a process's memory mappings
> is allowed to change significantly between kernel versions,
> C library versions, and operating system releases.
> -Furthermore, this option is extremely hazardous (when used on its own),
> -because it forcibly removes preexisting mappings,
> +This option should only be used when the specified memory region has
> +already been reserved using another mapping; otherwise, it is extremely
> +hazardous because it forcibly removes preexisting mappings,
> making it easy for a multithreaded process to corrupt its own address space.
Yes, that's clearer and provides more information than before.
> .IP
> For example, suppose that thread A looks through
> @@ -284,13 +285,15 @@ and the PAM libraries
> .UR http://www.linux-pam.org
> .UE .
> .IP
> -Newer kernels
> -(Linux 4.17 and later) have a
> +For cases in which the specified memory region has not been reserved using an
> +existing mapping, newer kernels (Linux 4.17 and later) provide an option
> .B MAP_FIXED_NOREPLACE
> -option that avoids the corruption problem; if available,
> -.B MAP_FIXED_NOREPLACE
> -should be preferred over
> -.BR MAP_FIXED .
> +that should be used instead; older kernels require the caller to use
> +.I addr
> +as a hint (without
> +.BR MAP_FIXED )
Here, I got lost: the sentence suddenly jumps into explaining non-MAP_FIXED
behavior, in the MAP_FIXED section. Maybe if you break up the sentence, and
possibly omit non-MAP_FIXED discussion, it will help.
> +and take appropriate action if the kernel places the new mapping at a
> +different address.
> .TP
> .BR MAP_FIXED_NOREPLACE " (since Linux 4.17)"
> .\" commit a4ff8e8620d3f4f50ac4b41e8067b7d395056843
>
thanks,
--
John Hubbard
NVIDIA
^ permalink raw reply
* Re: [PATCH] mmap.2: MAP_FIXED is okay if the address range has been reserved
From: Michael Kerrisk (man-pages) @ 2018-04-12 18:32 UTC (permalink / raw)
To: Jann Horn, linux-man, mhocko, jhubbard, akpm, linux-mm,
linux-kernel, linux-api
Cc: mtk.manpages
In-Reply-To: <20180412153941.170849-1-jannh@google.com>
Hello Jann,
On 04/12/2018 05:39 PM, Jann Horn wrote:
> Clarify that MAP_FIXED is appropriate if the specified address range has
> been reserved using an existing mapping, but shouldn't be used otherwise.
>
> Signed-off-by: Jann Horn <jannh@google.com>
> ---
> man2/mmap.2 | 19 +++++++++++--------
> 1 file changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/man2/mmap.2 b/man2/mmap.2
> index bef8b4432..80c9ec285 100644
> --- a/man2/mmap.2
> +++ b/man2/mmap.2
> @@ -253,8 +253,9 @@ Software that aspires to be portable should use this option with care,
> keeping in mind that the exact layout of a process's memory mappings
> is allowed to change significantly between kernel versions,
> C library versions, and operating system releases.
> -Furthermore, this option is extremely hazardous (when used on its own),
> -because it forcibly removes preexisting mappings,
> +This option should only be used when the specified memory region has
> +already been reserved using another mapping; otherwise, it is extremely
> +hazardous because it forcibly removes preexisting mappings,
> making it easy for a multithreaded process to corrupt its own address space.
> .IP
> For example, suppose that thread A looks through
> @@ -284,13 +285,15 @@ and the PAM libraries
> .UR http://www.linux-pam.org
> .UE .
> .IP
> -Newer kernels
> -(Linux 4.17 and later) have a
> +For cases in which the specified memory region has not been reserved using an
> +existing mapping, newer kernels (Linux 4.17 and later) provide an option
> .B MAP_FIXED_NOREPLACE
> -option that avoids the corruption problem; if available,
> -.B MAP_FIXED_NOREPLACE
> -should be preferred over
> -.BR MAP_FIXED .
> +that should be used instead; older kernels require the caller to use
> +.I addr
> +as a hint (without
> +.BR MAP_FIXED )
> +and take appropriate action if the kernel places the new mapping at a
> +different address.
> .TP
> .BR MAP_FIXED_NOREPLACE " (since Linux 4.17)"
> .\" commit a4ff8e8620d3f4f50ac4b41e8067b7d395056843
Thanks! Nice patch! Applied.
Cheers,
Michael
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
^ permalink raw reply
* Re: [PATCH 01/24] Add the ability to lock down access to the running kernel image
From: Linus Torvalds @ 2018-04-12 16:52 UTC (permalink / raw)
To: Justin Forbes
Cc: Jordan Glover, David Howells, linux-man, Linux API, James Morris,
Linux Kernel Mailing List, LSM List
In-Reply-To: <CAFxkdAr5c_bKEKqV2t-PKAo3iSvEkpqO1p+RPg9t-sW4yEUOnw@mail.gmail.com>
On Thu, Apr 12, 2018 at 6:09 AM, Justin Forbes <jmforbes@linuxtx.org> wrote:
> On Wed, Apr 11, 2018, 5:38 PM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>>
>> So it's really the whole claim that distributions have been running
>> for this for the last five years that I wonder about, and how often
>> people end up being told: "just disable secure boot":.
>
> Very rarely in my experience.
Good. Do you have a handle on the reasons?
Because I'm assuming it's not /dev/{mem,kmem,port}? Because I'd really
be happier if we just say "those are legacy, don't enable them at all
for modern distros".
That way they'd _stay_ disabled even if somebody cannot handle the
other limitations, like DMA etc.
Linus
^ permalink raw reply
* [PATCH] mmap.2: MAP_FIXED is okay if the address range has been reserved
From: Jann Horn @ 2018-04-12 15:39 UTC (permalink / raw)
To: mtk.manpages, jannh
Cc: linux-man, Michal Hocko, John Hubbard, Andrew Morton, linux-mm,
linux-kernel, linux-api
Clarify that MAP_FIXED is appropriate if the specified address range has
been reserved using an existing mapping, but shouldn't be used otherwise.
Signed-off-by: Jann Horn <jannh@google.com>
---
man2/mmap.2 | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/man2/mmap.2 b/man2/mmap.2
index bef8b4432..80c9ec285 100644
--- a/man2/mmap.2
+++ b/man2/mmap.2
@@ -253,8 +253,9 @@ Software that aspires to be portable should use this option with care,
keeping in mind that the exact layout of a process's memory mappings
is allowed to change significantly between kernel versions,
C library versions, and operating system releases.
-Furthermore, this option is extremely hazardous (when used on its own),
-because it forcibly removes preexisting mappings,
+This option should only be used when the specified memory region has
+already been reserved using another mapping; otherwise, it is extremely
+hazardous because it forcibly removes preexisting mappings,
making it easy for a multithreaded process to corrupt its own address space.
.IP
For example, suppose that thread A looks through
@@ -284,13 +285,15 @@ and the PAM libraries
.UR http://www.linux-pam.org
.UE .
.IP
-Newer kernels
-(Linux 4.17 and later) have a
+For cases in which the specified memory region has not been reserved using an
+existing mapping, newer kernels (Linux 4.17 and later) provide an option
.B MAP_FIXED_NOREPLACE
-option that avoids the corruption problem; if available,
-.B MAP_FIXED_NOREPLACE
-should be preferred over
-.BR MAP_FIXED .
+that should be used instead; older kernels require the caller to use
+.I addr
+as a hint (without
+.BR MAP_FIXED )
+and take appropriate action if the kernel places the new mapping at a
+different address.
.TP
.BR MAP_FIXED_NOREPLACE " (since Linux 4.17)"
.\" commit a4ff8e8620d3f4f50ac4b41e8067b7d395056843
--
2.17.0.484.g0c8726318c-goog
^ permalink raw reply related
* Re: [PATCH 24/24] debugfs: Restrict debugfs when the kernel is locked down
From: Andy Lutomirski @ 2018-04-12 14:19 UTC (permalink / raw)
To: Greg KH
Cc: Andy Lutomirski, David Howells, Linus Torvalds, linux-man,
Linux API, James Morris, LKML, LSM List
In-Reply-To: <20180412082313.GA6054@kroah.com>
On Thu, Apr 12, 2018 at 1:23 AM, Greg KH <greg@kroah.com> wrote:
> On Wed, Apr 11, 2018 at 07:54:12PM -0700, Andy Lutomirski wrote:
>> On Wed, Apr 11, 2018 at 1:33 PM, Greg KH <greg@kroah.com> wrote:
>> > 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.
>> >
>>
>> Debugfs is very, very useful for, ahem, debugging. I really think
>> this is an example of why we should split lockdown into the read and
>> write varieties and allow mounting and reading debugfs when only write
>> is locked down.
>
> Ok, but be sure that there are no "secrets" in those debugging files if
> you really buy into the whole "lock down" mess...
>
> Really, it's easier to just disable the whole thing.
>
I mostly agree with your sentiment. I'm saying that, for most uses, I
*don't* buy into the idea that a normal secure-boot-supporting distro
should block debugfs. I sometimes like to ask people who report
problems to send me the contents of such-and-such file in debugfs, and
I think it should keep working. Blocking write access to debugfs is
mostly sensible for a lockdown system, but blocking read only makes
sense if you're worried about straight-up bugs or if you think that
debugfs contains protection-worthy secrets.
What I want to see is:
lockdown=protect_integrity: debugfs is read-only, bpf and perf are
unrestricted, iopl and ioperm are disabled, etc.
lockdown=protect_integrity_and_secrecy: debugfs is gone, bpf and perf
are restricted, plus all the restrictions from
lockdown=protect_integrity
Distros should strongly prefer lockdown=protect_integrity (or
lockdown=off) by default. lockdown=protect_integrity_and_secrecy is
for custom setups, embedded applications, etc.
--Andy
^ permalink raw reply
* Re: [PATCH 01/24] Add the ability to lock down access to the running kernel image
From: Justin Forbes @ 2018-04-12 13:09 UTC (permalink / raw)
To: Linus Torvalds
Cc: Jordan Glover, David Howells, linux-man, Linux API, James Morris,
Linux Kernel Mailing List, LSM List
In-Reply-To: <CA+55aFzvnf=4OeBy5vQQ6HQoCsgCkAJw1LhSBOTkDWm3ck1pZA@mail.gmail.com>
On Wed, Apr 11, 2018, 5:38 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Wed, Apr 11, 2018 at 2:05 PM, Jordan Glover
> <Golden_Miller83@protonmail.ch> wrote:
> >>
> >> 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.
> >
> > CONFIG_DEVMEM=n
>
> It's actually CONFIG_DEVMEM, CONFIG_DEVKMEM and CONFIG_DEVPORT, it's
> just not obvious from the patch.
>
> But the important part is this part:
>
> >> 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.
>
> .. because I get the feeling that not a lot of people have actually
> been testing this, because "turn off secure boot" is such a universal
> thing when people boot Linux.
>
> So it's really the whole claim that distributions have been running
> for this for the last five years that I wonder about, and how often
> people end up being told: "just disable secure boot":.
Very rarely in my experience. And the one time that we sent a kernel
to updates-testing that was signed with the test key instead of the
real key, we had a surprisingly high number of reports from users that
it was broken before the update even got synched to mirrors. So we
don't have actual numbers of users running active secure boot with
Fedora, but we do know it is more than we expected. The majority of
people who do run into issues are those running out of tree modules,
who haven't imported any sort of key for local signing. This isn't
like SELinux was at launch where it was so invasive that a large
number of users instinctively turned it off with every installation, I
would guess even people who turned it off in the past, don't even
think about it when they get a new machine and leave it on.
> But if people really don't need DEVMEM/DEVKMEM/DEVPORT, maybe we
> should just disable them in the default configs, and consider them
> legacy.
>
> I'm just surprised. I suspect a lot of people end up actually using
> devmem as a fallback for dmidecode etc. Maybe those people don't boot
> with EFI secure mode, but if so that just shows that this whole
> "hardening" is just security theater.
>
> Linus
^ permalink raw reply
* Re: [PATCH] getcwd.3: Mention that "(unreachable)" is no longer returned for glibc > 2.27.
From: Michael Kerrisk (man-pages) @ 2018-04-12 11:50 UTC (permalink / raw)
To: Carlos O'Donell, linux-man@vger.kernel.org, GNU C Library
Cc: mtk.manpages, Florian Weimer
In-Reply-To: <94a3f8a3-5c0e-9fa5-ce2c-3f7e4b097a97@redhat.com>
Hello Carlos,
On 02/05/2018 09:34 PM, Carlos O'Donell wrote:
> Michael,
>
> With glibc fix 52a713fdd0a30e1bd79818e2e3c4ab44ddca1a94 for
> CVE-2018-1000001 (Sourceware BZ #22679) the implementation in the
> just released glibc 2.27 has been changed such that instead of
> returning "(unreachable)" the implementation now returns ENOENT
> as it would have if the current directory had been unlinked.
>
> I see that in 2015 the quirk was documented in commit
> a2ac97c78bf05a55f8f616fc39a4724372dcfa95, and this is no longer
> true with glibc 2.27, but may continue to be true in other C libraries,
> so I reference NOTES from the paragraph in the central text.
Thanks. Patch applied.
Cheers,
Michael
> Signed-off-by: Carlos O'Donell <carlos@redhat.com>
>
> diff --git a/man3/getcwd.3 b/man3/getcwd.3
> index ff953a7d0..3c41736c9 100644
> --- a/man3/getcwd.3
> +++ b/man3/getcwd.3
> @@ -91,7 +91,9 @@ the current directory into another mount namespace.
> When dealing with paths from untrusted sources, callers of these
> functions should consider checking whether the returned path starts
> with '/' or '(' to avoid misinterpreting an unreachable path
> -as a relative path.
> +as a relative path. This is no longer true under some C libraries,
> +see
> +.BR NOTES .
> .PP
> The
> .BR getcwd ()
> @@ -270,6 +272,16 @@ generic implementation is called.
> Only in that case can
> these calls fail under Linux with
> .BR EACCES .
> +.PP
> +Since Linux commit v2.6.36 which added "(unreachable)" the glibc
> +.BR getcwd ()
> +has failed to conform to POSIX and returned a relative path when the API
> +contract requires an absolute path. With glibc 2.27 onwards this is corrected;
> +calling
> +.BR getcwd ()
> +from such a path will now result in failure with
> +.BR ENOENT .
> +
> .PP
> These functions are often used to save the location of the current working
> directory for the purpose of returning to it later.
> ---
>
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
^ permalink raw reply
* Re: [PATCH 06/24] kexec_load: Disable at runtime if the kernel is locked down
From: Mimi Zohar @ 2018-04-12 11:38 UTC (permalink / raw)
To: Eric W. Biederman, David Howells
Cc: torvalds, linux-man, linux-api, jmorris, linux-kernel,
linux-security-module
In-Reply-To: <1523477375.5268.78.camel@linux.vnet.ibm.com>
On Wed, 2018-04-11 at 16:09 -0400, Mimi Zohar wrote:
> 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().
David, enabling the IMA-appraisal "secure boot" policy should probably
not be dependent on lockdown either.
Mimi
^ permalink raw reply
* Re: [PATCH v2] readv.2, io_submit.2: Document RWF_APPEND added in Linux 4.16
From: Michael Kerrisk (man-pages) @ 2018-04-12 9:11 UTC (permalink / raw)
To: Jürg Billeter; +Cc: mtk.manpages, linux-man, linux-kernel
In-Reply-To: <20180406135106.30342-1-j@bitron.ch>
On 04/06/2018 03:51 PM, Jürg Billeter wrote:
> Signed-off-by: Jürg Billeter <j@bitron.ch>
Thanks, Jürg. Patch applied.
Cheers,
Michael
> ---
> Changes since version 1:
> - Explain offset handling
>
> man2/io_submit.2 | 13 +++++++++++++
> man2/readv.2 | 17 +++++++++++++++++
> 2 files changed, 30 insertions(+)
>
> diff --git a/man2/io_submit.2 b/man2/io_submit.2
> index 397fd0b75..25961138a 100644
> --- a/man2/io_submit.2
> +++ b/man2/io_submit.2
> @@ -111,6 +111,19 @@ field of the
> .I io_event
> structure (see
> .BR io_getevents (2)).
> +.TP
> +.BR RWF_APPEND " (since Linux 4.16)"
> +.\" commit e1fc742e14e01d84d9693c4aca4ab23da65811fb
> +Append data to the end of the file.
> +See the description of the flag of the same name in
> +.BR pwritev2 (2)
> +as well as the description of
> +.B O_APPEND
> +in
> +.BR open (2).
> +The
> +.I aio_offset
> +field is ignored. The file offset is not changed.
> .RE
> .TP
> .I aio_lio_opcode
> diff --git a/man2/readv.2 b/man2/readv.2
> index b3b7b9658..9ef250e11 100644
> --- a/man2/readv.2
> +++ b/man2/readv.2
> @@ -248,6 +248,23 @@ to
> .BR EAGAIN .
> Currently, this flag is meaningful only for
> .BR preadv2 ().
> +.TP
> +.BR RWF_APPEND " (since Linux 4.16)"
> +.\" commit e1fc742e14e01d84d9693c4aca4ab23da65811fb
> +Provide a per-write equivalent of the
> +.B O_APPEND
> +.BR open (2)
> +flag.
> +This flag is meaningful only for
> +.BR pwritev2 (),
> +and its effect applies only to the data range written by the system call.
> +The
> +.I offset
> +argument does not affect the write operation, the data is always appended
> +to the end of the file. However, if the
> +.I offset
> +argument is \-1, the current file offset is updated.
> +This matches the behavior when the file is opened in append mode.
> .SH RETURN VALUE
> On success,
> .BR readv (),
>
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
^ permalink raw reply
* Re: [PATCH 24/24] debugfs: Restrict debugfs when the kernel is locked down
From: Greg KH @ 2018-04-12 8:23 UTC (permalink / raw)
To: Andy Lutomirski
Cc: David Howells, Linus Torvalds, linux-man, Linux API, James Morris,
LKML, LSM List
In-Reply-To: <CALCETrWynogzZcaKMLWKce1rMvAC_sViReBDyiEWL-rm21HvAg@mail.gmail.com>
On Wed, Apr 11, 2018 at 07:54:12PM -0700, Andy Lutomirski wrote:
> On Wed, Apr 11, 2018 at 1:33 PM, Greg KH <greg@kroah.com> wrote:
> > 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.
> >
>
> Debugfs is very, very useful for, ahem, debugging. I really think
> this is an example of why we should split lockdown into the read and
> write varieties and allow mounting and reading debugfs when only write
> is locked down.
Ok, but be sure that there are no "secrets" in those debugging files if
you really buy into the whole "lock down" mess...
Really, it's easier to just disable the whole thing.
greg k-h
^ permalink raw reply
* Re: [PATCH 01/24] Add the ability to lock down access to the running kernel image
From: Andy Lutomirski @ 2018-04-12 2:57 UTC (permalink / raw)
To: David Howells
Cc: Linus Torvalds, linux-man, Linux API, James Morris, LKML,
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:
>
> (*) 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
By doing this, you are basically committing to making the
protect-kernel-integrity vs protect-kernel-secrecy split be a
second-class citizen if it gets added.
How about lockdown=integrity_and_secrecy or lockdown=2 if you feel
like using numbers?
^ permalink raw reply
* Re: [PATCH 24/24] debugfs: Restrict debugfs when the kernel is locked down
From: Andy Lutomirski @ 2018-04-12 2:54 UTC (permalink / raw)
To: Greg KH
Cc: David Howells, Linus Torvalds, linux-man, Linux API, James Morris,
LKML, LSM List
In-Reply-To: <20180411203308.GA10167@kroah.com>
On Wed, Apr 11, 2018 at 1:33 PM, Greg KH <greg@kroah.com> wrote:
> 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.
>
Debugfs is very, very useful for, ahem, debugging. I really think
this is an example of why we should split lockdown into the read and
write varieties and allow mounting and reading debugfs when only write
is locked down.
^ 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 22:38 UTC (permalink / raw)
To: Jordan Glover
Cc: David Howells, linux-man, Linux API, James Morris,
Linux Kernel Mailing List, LSM List
In-Reply-To: <8z0aRQyD-6Krqntk8UD9WQjK5JSqEai2Pt5oeFU2EplgxoWiHlX5nlJXwCDHQ1WcS1oIprXimgz7UvwHCWDB9Z3dYFrEmZmtkEJSqaYMel8=@protonmail.ch>
On Wed, Apr 11, 2018 at 2:05 PM, Jordan Glover
<Golden_Miller83@protonmail.ch> wrote:
>>
>> 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.
>
> CONFIG_DEVMEM=n
It's actually CONFIG_DEVMEM, CONFIG_DEVKMEM and CONFIG_DEVPORT, it's
just not obvious from the patch.
But the important part is this part:
>> 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.
.. because I get the feeling that not a lot of people have actually
been testing this, because "turn off secure boot" is such a universal
thing when people boot Linux.
So it's really the whole claim that distributions have been running
for this for the last five years that I wonder about, and how often
people end up being told: "just disable secure boot":.
But if people really don't need DEVMEM/DEVKMEM/DEVPORT, maybe we
should just disable them in the default configs, and consider them
legacy.
I'm just surprised. I suspect a lot of people end up actually using
devmem as a fallback for dmidecode etc. Maybe those people don't boot
with EFI secure mode, but if so that just shows that this whole
"hardening" is just security theater.
Linus
^ permalink raw reply
* Re: [PATCH 01/24] Add the ability to lock down access to the running kernel image
From: Jordan Glover @ 2018-04-11 21:05 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 April 11, 2018 8: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.
CONFIG_DEVMEM=n
>
> 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
>
Jordan
^ 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
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox