Re: BTI interaction between seccomp filters in systemd and glibc mprotect calls, causing service failures

2020-10-25 Thread Jordan Glover
On Saturday, October 24, 2020 2:12 PM, Salvatore Mesoraca 
 wrote:

> On Sat, 24 Oct 2020 at 12:34, Topi Miettinen toiwo...@gmail.com wrote:
>
> > On 23.10.2020 20.52, Salvatore Mesoraca wrote:
> >
> > > Hi,
> > > On Thu, 22 Oct 2020 at 23:24, Topi Miettinen toiwo...@gmail.com wrote:
> > >
> > > > SARA looks interesting. What is missing is a prctl() to enable all W^X
> > > > protections irrevocably for the current process, then systemd could
> > > > enable it for services with MemoryDenyWriteExecute=yes.
> > >
> > > SARA actually has a procattr[0] interface to do just that.
> > > There is also a library[1] to help using it.
> >
> > That means that /proc has to be available and writable at that point, so
> > setting up procattrs has to be done before mount namespaces are set up.
> > In general, it would be nice for sandboxing facilities in kernel if
> > there would be a way to start enforcing restrictions only at next
> > execve(), like setexeccon() for SELinux and aa_change_onexec() for
> > AppArmor. Otherwise the exact order of setting up various sandboxing
> > options can be very tricky to arrange correctly, since each option may
> > have a subtle effect to the sandboxing features enabled later. In case
> > of SARA, the operations done between shuffling the mount namespace and
> > before execve() shouldn't be affected so it isn't important. Even if it
> > did (a new sandboxing feature in the future would need trampolines or
> > JIT code generation), maybe the procattr file could be opened early but
> > it could be written closer to execve().
>
> A new "apply on exec" procattr file seems reasonable and relatively easy to 
> add.
> As Kees pointed out, the main obstacle here is the fact that SARA is
> not upstream :(
>
> Salvatore

Is there a chance we will see new SARA iteration soon on lkml? :)

Jordan


Re: [PATCH v5 00/12] S.A.R.A. a new stacked LSM

2019-07-06 Thread Jordan Glover
On Saturday, July 6, 2019 10:54 AM, Salvatore Mesoraca  
wrote:

> S.A.R.A. is meant to be stacked but it needs cred blobs and the procattr
> interface, so I temporarily implemented those parts in a way that won't
> be acceptable for upstream, but it works for now. I know that there
> is some ongoing work to make cred blobs and procattr stackable, as soon
> as the new interfaces will be available I'll reimplement the involved
> parts.

I thought all stacking pieces for minor LSM were merged in Linux 5.1.
Is there still something missing or is this comment out-fo-date?

Jordan


Re: [RFC PATCH v1 0/5] Add support for O_MAYEXEC

2018-12-12 Thread Jordan Glover
On Wednesday, December 12, 2018 9:17 AM, Mickaël Salaün  
wrote:

> Hi,
>
> The goal of this patch series is to control script interpretation. A
> new O_MAYEXEC flag used by sys_open() is added to enable userland script
> interpreter to delegate to the kernel (and thus the system security
> policy) the permission to interpret scripts or other files containing
> what can be seen as commands.
>
> The security policy is the responsibility of an LSM. A basic
> system-wide policy is implemented with Yama and configurable through a
> sysctl.
>
> The initial idea come from CLIP OS and the original implementation has
> been used for more than 10 years:
> https://github.com/clipos-archive/clipos4_doc
>
> An introduction to O_MAYEXEC was given at the Linux Security Summit
> Europe 2018 - Linux Kernel Security Contributions by ANSSI:
> https://www.youtube.com/watch?v=chNjCRtPKQY=17m15s
> The "write xor execute" principle was explained at Kernel Recipes 2018 -
> CLIP OS: a defense-in-depth OS:
> https://www.youtube.com/watch?v=PjRE0uBtkHU=11m14s
>
> This patch series can be applied on top of v4.20-rc6. This can be
> tested with CONFIG_SECURITY_YAMA. I would really appreciate
> constructive comments on this RFC.
>
> Regards,
>

Are various interpreters upstreams interested in adding support
for O_MAYEXEC if it land in kernel? Did you contacted them about this?

Jordan



Re: [PATCH v1 2/2]: Documentation/admin-guide: introduce perf-security.rst file

2018-11-19 Thread Jordan Glover
On Monday, November 19, 2018 11:46 AM, Peter Zijlstra  
wrote:

> On Mon, Nov 19, 2018 at 10:35:59AM +0000, Jordan Glover wrote:
>
> > On Monday, November 19, 2018 6:42 AM, Alexey Budankov 
> > alexey.budan...@linux.intel.com wrote:
> >
> > > +>=3:
> > >
> > > - Restrict *access* to PCL performance monitoring for 
> > > unprivileged processes.
> > >
> > >
> > > - This is the default on Debian and Android [7]_ , [8]_ .
> > >
> > >
> >
> > AFAIK there is no support for '+>=3' in mainline kernel[1].
> > Debian and Android use out-of-tree patch for that[2].
> > Maybe someone should upstream it?
>
> NAK still stands on that. Alternative's have been proposed but so far
> nobody that cared seems to care enough to implement those.

So, I guess we can't document NAKed patches :)

Jordan



Re: [PATCH v1 2/2]: Documentation/admin-guide: introduce perf-security.rst file

2018-11-19 Thread Jordan Glover
On Monday, November 19, 2018 11:46 AM, Peter Zijlstra  
wrote:

> On Mon, Nov 19, 2018 at 10:35:59AM +0000, Jordan Glover wrote:
>
> > On Monday, November 19, 2018 6:42 AM, Alexey Budankov 
> > alexey.budan...@linux.intel.com wrote:
> >
> > > +>=3:
> > >
> > > - Restrict *access* to PCL performance monitoring for 
> > > unprivileged processes.
> > >
> > >
> > > - This is the default on Debian and Android [7]_ , [8]_ .
> > >
> > >
> >
> > AFAIK there is no support for '+>=3' in mainline kernel[1].
> > Debian and Android use out-of-tree patch for that[2].
> > Maybe someone should upstream it?
>
> NAK still stands on that. Alternative's have been proposed but so far
> nobody that cared seems to care enough to implement those.

So, I guess we can't document NAKed patches :)

Jordan



Re: [PATCH v1 2/2]: Documentation/admin-guide: introduce perf-security.rst file

2018-11-19 Thread Jordan Glover
On Monday, November 19, 2018 6:42 AM, Alexey Budankov 
 wrote:

> Implement initial version of perf-security.rst documentation file
> initially covering security concerns related to PCL/Perf performance
> monitoring in multiuser environments.
>
> Suggested-by: Thomas Gleixner t...@linutronix.de
> Signed-off-by: Alexey Budankov alexey.budan...@linux.intel.com
>
> Documentation/admin-guide/perf-security.rst | 83 +
> 1 file changed, 83 insertions(+)
>
> diff --git a/Documentation/admin-guide/perf-security.rst 
> b/Documentation/admin-guide/perf-security.rst
> new file mode 100644
> index ..b9564066e686
> --- /dev/null
> +++ b/Documentation/admin-guide/perf-security.rst
> @@ -0,0 +1,83 @@
> +.. perf_security:
> +
> +PCL/Perf security
> +=
> +
> +Overview
> +
> +
> +Usage of Performance Counters for Linux (PCL) [1] , [2]_ , [3]_ can impose 
> a+considerable risk of leaking sensitive data accessed by monitored processes.
> +The data leakage is possible both in scenarios of direct usage of PCL system
> +call API [2]_ and over data files generated by Perf tool user mode utility
> +(Perf) [3]_ , [4]_ . The risk depends on the nature of data that PCL 
> performance
> +monitoring units (PMU) [2]_ collect and expose for performance analysis.
> +Having that said PCL/Perf performance monitoring is the subject for security
> +access control management [5]_ .
> +
> +PCL/Perf access control
> +---
> +
> +For the purpose of performing security checks Linux implementation splits
> +processes into two categories [6]_ : a) privileged processes (whose effective
> +user ID is 0, referred to as superuser or root), and b) unprivileged 
> processes
> +(whose effective UID is nonzero). Privileged processes bypass all kernel
> +security permission checks so PCL performance monitoring is fully available 
> to
> +privileged processes without access, scope and resource restrictions.
> +Unprivileged processes are subject to full security permission check based
> +on the process's credentials [5]_ (usually: effective UID, effective GID,
> +and supplementary group list).
> +
> +PCL/Perf unprivileged users
> +---
> +
> +PCL/Perf scope and access control for unprivileged processes is governed by
> +perf_event_paranoid [2]_ setting:
> +
> +-1:
>
> -   Impose no *scope* and *access* restrictions on using PCL performance
>
>
> -   monitoring. Per-user per-cpu perf_event_mlock_kb [2]_ locking limit is
>
>
> -   ignored when allocating memory buffers for storing performance data.
>
>
> -   This is the least secure mode since allowed monitored *scope* is
>
>
> -   maximized and no PCL specific limits are imposed on *resources*
>
>
> -   allocated for performance monitoring.
>
>
> -
>
> +>=0:
>
> -   *scope* includes per-process and system wide performance monitoring
>
>
> -   but excludes raw tracepoints and ftrace function tracepoints 
> monitoring.
>
>
> -   CPU and system events happened when executing either in user or
>
>
> -   in kernel space can be monitored and captured for later analysis.
>
>
> -   Per-user per-cpu perf_event_mlock_kb locking limit is imposed but
>
>
> -   ignored for unprivileged processes with CAP_IPC_LOCK [6]_ capability.
>
>
> -
>
> +>=1:
>
> -   *scope* includes per-process performance monitoring only and excludes
>
>
> -   system wide performance monitoring. CPU and system events happened 
> when
>
>
> -   executing either in user or in kernel space can be monitored and
>
>
> -   captured for later analysis. Per-user per-cpu perf_event_mlock_kb
>
>
> -   locking limit is imposed but ignored for unprivileged processes with
>
>
> -   CAP_IPC_LOCK capability.
>
>
> -
>
> +>=2:
>
> -   *scope* includes per-process performance monitoring only. CPU and 
> system
>
>
> -   events happened when executing in user space only can be monitored and
>
>
> -   captured for later analysis. Per-user per-cpu perf_event_mlock_kb
>
>
> -   locking limit is imposed but ignored for unprivileged processes with
>
>
> -   CAP_IPC_LOCK capability.
>
>
> -
>
> +>=3:
>
> -   Restrict *access* to PCL performance monitoring for unprivileged 
> processes.
>
>
> -   This is the default on Debian and Android [7]_ , [8]_ .

AFAIK there is no support for '+>=3' in mainline kernel[1].
Debian and Android use out-of-tree patch for that[2].
Maybe someone should upstream it?

Jordan

[1] https://github.com/torvalds/linux/blob/master/kernel/events/core.c#L395
[2] 
https://salsa.debian.org/kernel-team/linux/blob/master/debian/patches/features/all/security-perf-allow-further-restriction-of-perf_event_open.patch


Re: [PATCH v1 2/2]: Documentation/admin-guide: introduce perf-security.rst file

2018-11-19 Thread Jordan Glover
On Monday, November 19, 2018 6:42 AM, Alexey Budankov 
 wrote:

> Implement initial version of perf-security.rst documentation file
> initially covering security concerns related to PCL/Perf performance
> monitoring in multiuser environments.
>
> Suggested-by: Thomas Gleixner t...@linutronix.de
> Signed-off-by: Alexey Budankov alexey.budan...@linux.intel.com
>
> Documentation/admin-guide/perf-security.rst | 83 +
> 1 file changed, 83 insertions(+)
>
> diff --git a/Documentation/admin-guide/perf-security.rst 
> b/Documentation/admin-guide/perf-security.rst
> new file mode 100644
> index ..b9564066e686
> --- /dev/null
> +++ b/Documentation/admin-guide/perf-security.rst
> @@ -0,0 +1,83 @@
> +.. perf_security:
> +
> +PCL/Perf security
> +=
> +
> +Overview
> +
> +
> +Usage of Performance Counters for Linux (PCL) [1] , [2]_ , [3]_ can impose 
> a+considerable risk of leaking sensitive data accessed by monitored processes.
> +The data leakage is possible both in scenarios of direct usage of PCL system
> +call API [2]_ and over data files generated by Perf tool user mode utility
> +(Perf) [3]_ , [4]_ . The risk depends on the nature of data that PCL 
> performance
> +monitoring units (PMU) [2]_ collect and expose for performance analysis.
> +Having that said PCL/Perf performance monitoring is the subject for security
> +access control management [5]_ .
> +
> +PCL/Perf access control
> +---
> +
> +For the purpose of performing security checks Linux implementation splits
> +processes into two categories [6]_ : a) privileged processes (whose effective
> +user ID is 0, referred to as superuser or root), and b) unprivileged 
> processes
> +(whose effective UID is nonzero). Privileged processes bypass all kernel
> +security permission checks so PCL performance monitoring is fully available 
> to
> +privileged processes without access, scope and resource restrictions.
> +Unprivileged processes are subject to full security permission check based
> +on the process's credentials [5]_ (usually: effective UID, effective GID,
> +and supplementary group list).
> +
> +PCL/Perf unprivileged users
> +---
> +
> +PCL/Perf scope and access control for unprivileged processes is governed by
> +perf_event_paranoid [2]_ setting:
> +
> +-1:
>
> -   Impose no *scope* and *access* restrictions on using PCL performance
>
>
> -   monitoring. Per-user per-cpu perf_event_mlock_kb [2]_ locking limit is
>
>
> -   ignored when allocating memory buffers for storing performance data.
>
>
> -   This is the least secure mode since allowed monitored *scope* is
>
>
> -   maximized and no PCL specific limits are imposed on *resources*
>
>
> -   allocated for performance monitoring.
>
>
> -
>
> +>=0:
>
> -   *scope* includes per-process and system wide performance monitoring
>
>
> -   but excludes raw tracepoints and ftrace function tracepoints 
> monitoring.
>
>
> -   CPU and system events happened when executing either in user or
>
>
> -   in kernel space can be monitored and captured for later analysis.
>
>
> -   Per-user per-cpu perf_event_mlock_kb locking limit is imposed but
>
>
> -   ignored for unprivileged processes with CAP_IPC_LOCK [6]_ capability.
>
>
> -
>
> +>=1:
>
> -   *scope* includes per-process performance monitoring only and excludes
>
>
> -   system wide performance monitoring. CPU and system events happened 
> when
>
>
> -   executing either in user or in kernel space can be monitored and
>
>
> -   captured for later analysis. Per-user per-cpu perf_event_mlock_kb
>
>
> -   locking limit is imposed but ignored for unprivileged processes with
>
>
> -   CAP_IPC_LOCK capability.
>
>
> -
>
> +>=2:
>
> -   *scope* includes per-process performance monitoring only. CPU and 
> system
>
>
> -   events happened when executing in user space only can be monitored and
>
>
> -   captured for later analysis. Per-user per-cpu perf_event_mlock_kb
>
>
> -   locking limit is imposed but ignored for unprivileged processes with
>
>
> -   CAP_IPC_LOCK capability.
>
>
> -
>
> +>=3:
>
> -   Restrict *access* to PCL performance monitoring for unprivileged 
> processes.
>
>
> -   This is the default on Debian and Android [7]_ , [8]_ .

AFAIK there is no support for '+>=3' in mainline kernel[1].
Debian and Android use out-of-tree patch for that[2].
Maybe someone should upstream it?

Jordan

[1] https://github.com/torvalds/linux/blob/master/kernel/events/core.c#L395
[2] 
https://salsa.debian.org/kernel-team/linux/blob/master/debian/patches/features/all/security-perf-allow-further-restriction-of-perf_event_open.patch


Re: [patch V3 05/11] x86/mm/cpa: Add debug mechanism

2018-11-10 Thread Jordan Glover
Hello,

The patch  "x86/mm/cpa: Add debug mechanism" 
(https://lore.kernel.org/lkml/20180917143546.078998...@linutronix.de/) caused a 
thousands of messages during boot on my machine.

They look like below:

kernel: CPA  protectText RO: 0x8affc000 - 0x8affcfff PFN 
12cffc req 0063 prevent 0002
kernel: CPA  protectText RO: 0x8affd000 - 0x8affdfff PFN 
12cffd req 0063 prevent 0002
kernel: CPA  protectText RO: 0x8affe000 - 0x8affefff PFN 
12cffe req 0063 prevent 0002
kernel: CPA  protectText RO: 0x8afff000 - 0x8aff PFN 
12cfff req 0063 prevent 0002
kernel: CPA conflict  Rodata RO: 0xa2fbece0 - 0xa2fbecff PFN 
12ce00 req 80e3 prevent 0002
kernel: CPA  protect  Rodata RO: 0xa2fbece0 - 0xa2fbece00fff PFN 
12ce00 req 8063 prevent 0002
kernel: CPA  protect  Rodata RO: 0xa2fbece01000 - 0xa2fbece01fff PFN 
12ce01 req 8063 prevent 0002
kernel: CPA  protect  Rodata RO: 0xa2fbece02000 - 0xa2fbece02fff PFN 
12ce02 req 8063 prevent 0002


The exact reason for those may be related to some out-of-tree patches which I 
use:

https://github.com/anthraxx/linux-hardened/commit/c492687efa253c32972115a5e110e51614af445e#diff-e1f7e9a10ed7feb8c32493438cff8456

https://github.com/anthraxx/linux-hardened/commit/266d16fc47257b2548e6fcbfafebcc5d84d46389#diff-e1f7e9a10ed7feb8c32493438cff8456

but I wonder if those messages shouldn't be rate-limited or hidden behind 
'CONFIG_CPA_DEBUG' in general as there may be a chance that someone will 
trigger something like that with vanillia kernel.

Jordan


Re: [patch V3 05/11] x86/mm/cpa: Add debug mechanism

2018-11-10 Thread Jordan Glover
Hello,

The patch  "x86/mm/cpa: Add debug mechanism" 
(https://lore.kernel.org/lkml/20180917143546.078998...@linutronix.de/) caused a 
thousands of messages during boot on my machine.

They look like below:

kernel: CPA  protectText RO: 0x8affc000 - 0x8affcfff PFN 
12cffc req 0063 prevent 0002
kernel: CPA  protectText RO: 0x8affd000 - 0x8affdfff PFN 
12cffd req 0063 prevent 0002
kernel: CPA  protectText RO: 0x8affe000 - 0x8affefff PFN 
12cffe req 0063 prevent 0002
kernel: CPA  protectText RO: 0x8afff000 - 0x8aff PFN 
12cfff req 0063 prevent 0002
kernel: CPA conflict  Rodata RO: 0xa2fbece0 - 0xa2fbecff PFN 
12ce00 req 80e3 prevent 0002
kernel: CPA  protect  Rodata RO: 0xa2fbece0 - 0xa2fbece00fff PFN 
12ce00 req 8063 prevent 0002
kernel: CPA  protect  Rodata RO: 0xa2fbece01000 - 0xa2fbece01fff PFN 
12ce01 req 8063 prevent 0002
kernel: CPA  protect  Rodata RO: 0xa2fbece02000 - 0xa2fbece02fff PFN 
12ce02 req 8063 prevent 0002


The exact reason for those may be related to some out-of-tree patches which I 
use:

https://github.com/anthraxx/linux-hardened/commit/c492687efa253c32972115a5e110e51614af445e#diff-e1f7e9a10ed7feb8c32493438cff8456

https://github.com/anthraxx/linux-hardened/commit/266d16fc47257b2548e6fcbfafebcc5d84d46389#diff-e1f7e9a10ed7feb8c32493438cff8456

but I wonder if those messages shouldn't be rate-limited or hidden behind 
'CONFIG_CPA_DEBUG' in general as there may be a chance that someone will 
trigger something like that with vanillia kernel.

Jordan


Re: [PATCH security-next v4 23/32] selinux: Remove boot parameter

2018-10-04 Thread Jordan Glover
Sent with ProtonMail Secure Email.

‐‐‐ Original Message ‐‐‐
On Thursday, October 4, 2018 6:18 PM, Kees Cook  wrote:

>
> I don't want to overload "security=", but we can if we want. It would
> be as above, but a trailing comma would be needed to trigger the
> "ordering" behavior. e.g. "security=selinux" would disable all other
> majors (retaining the current behavior), but "security=selinux," would
> disable all other LSMs.
>
> -Kees
>
>

I don't think giving such big impact to trailing comma is good idea :)

Jordan


Re: [PATCH security-next v4 23/32] selinux: Remove boot parameter

2018-10-04 Thread Jordan Glover
Sent with ProtonMail Secure Email.

‐‐‐ Original Message ‐‐‐
On Thursday, October 4, 2018 6:18 PM, Kees Cook  wrote:

>
> I don't want to overload "security=", but we can if we want. It would
> be as above, but a trailing comma would be needed to trigger the
> "ordering" behavior. e.g. "security=selinux" would disable all other
> majors (retaining the current behavior), but "security=selinux," would
> disable all other LSMs.
>
> -Kees
>
>

I don't think giving such big impact to trailing comma is good idea :)

Jordan


Re: [PATCH] tracing: do not leak kernel addresses

2018-07-27 Thread Jordan Glover
On July 27, 2018 12:15 AM, Steven Rostedt  wrote:

> On Thu, 26 Jul 2018 09:52:11 -0700
> Nick Desaulniers ndesaulni...@google.com wrote:
>
> > See the section "Kernel addresses" in
> > Documentation/security/self-protection. IIRC, the issue is that a
> > process may have CAP_SYSLOG but not necessarily CAP_SYS_ADMIN (so it
> > can read dmesg, but not necessarily issue a sysctl to change
> > kptr_restrict), get compromised and used to leak kernel addresses,
> > which can then be used to defeat KASLR.
>
> But the code doesn't go to dmesg. It's only available
> via /sys/kernel/debug/tracing/printk_formats which is only available
> via root. Nobody else has access to that directory.
>
> -- Steve

I think the point was that when we take capabilities into account the root
privileges aren't unequivocal anymore. The 'root' owned process with only
'CAP_SYSLOG' shouldn't have access to /sys/kernel/debug/tracing/printk_formats

Jordan


Re: [PATCH] tracing: do not leak kernel addresses

2018-07-27 Thread Jordan Glover
On July 27, 2018 12:15 AM, Steven Rostedt  wrote:

> On Thu, 26 Jul 2018 09:52:11 -0700
> Nick Desaulniers ndesaulni...@google.com wrote:
>
> > See the section "Kernel addresses" in
> > Documentation/security/self-protection. IIRC, the issue is that a
> > process may have CAP_SYSLOG but not necessarily CAP_SYS_ADMIN (so it
> > can read dmesg, but not necessarily issue a sysctl to change
> > kptr_restrict), get compromised and used to leak kernel addresses,
> > which can then be used to defeat KASLR.
>
> But the code doesn't go to dmesg. It's only available
> via /sys/kernel/debug/tracing/printk_formats which is only available
> via root. Nobody else has access to that directory.
>
> -- Steve

I think the point was that when we take capabilities into account the root
privileges aren't unequivocal anymore. The 'root' owned process with only
'CAP_SYSLOG' shouldn't have access to /sys/kernel/debug/tracing/printk_formats

Jordan


Re: [PATCH v13 0/6] Introduce the STACKLEAK feature and a test for it

2018-06-24 Thread Jordan Glover
On June 24, 2018 9:18 AM, Ingo Molnar  wrote:

> -   Alexander Popov alex.po...@linux.com wrote:
> 
> > On 22.06.2018 06:16, Ingo Molnar wrote:
> > 
> > > -   Kees Cook keesc...@chromium.org wrote:
> > > 
> > > > On Thu, Jun 21, 2018 at 7:07 PM, Kees Cook keesc...@chromium.org wrote:
> > > > 
> > > > > It needs rebasing to -rc1 (which I've done already, but I'm still
> > > > > 
> > > > > doing build tests on it).
> > > > 
> > > > It's passed basic allmodconfig builds:
> > > > 
> > > > tree: https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git
> > > > 
> > > > branch: for-next/stackleak
> > > 
> > > Ok, that's 6 patches - please send the latest for review as emails to 
> > > lkml the
> > > 
> > > usual. Thanks!
> > 
> > I'll resend the rebased v13 shortly.
> 
> Well, I assume there were v12->v13 changes, correct?
> 
> If yes, and if it's on a different base krnel, then why did you call it a
> 
> 'resend'? That will only get fewer people to review it ... Please don't do 
> that
> 
> for future iterations.
> 
> Thanks,
> 
> Ingo

v13 was sent for the first time a month ago and you were on the list of 
recipients:
http://www.openwall.com/lists/kernel-hardening/2018/05/26/6

Jordan


Re: [PATCH v13 0/6] Introduce the STACKLEAK feature and a test for it

2018-06-24 Thread Jordan Glover
On June 24, 2018 9:18 AM, Ingo Molnar  wrote:

> -   Alexander Popov alex.po...@linux.com wrote:
> 
> > On 22.06.2018 06:16, Ingo Molnar wrote:
> > 
> > > -   Kees Cook keesc...@chromium.org wrote:
> > > 
> > > > On Thu, Jun 21, 2018 at 7:07 PM, Kees Cook keesc...@chromium.org wrote:
> > > > 
> > > > > It needs rebasing to -rc1 (which I've done already, but I'm still
> > > > > 
> > > > > doing build tests on it).
> > > > 
> > > > It's passed basic allmodconfig builds:
> > > > 
> > > > tree: https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git
> > > > 
> > > > branch: for-next/stackleak
> > > 
> > > Ok, that's 6 patches - please send the latest for review as emails to 
> > > lkml the
> > > 
> > > usual. Thanks!
> > 
> > I'll resend the rebased v13 shortly.
> 
> Well, I assume there were v12->v13 changes, correct?
> 
> If yes, and if it's on a different base krnel, then why did you call it a
> 
> 'resend'? That will only get fewer people to review it ... Please don't do 
> that
> 
> for future iterations.
> 
> Thanks,
> 
> Ingo

v13 was sent for the first time a month ago and you were on the list of 
recipients:
http://www.openwall.com/lists/kernel-hardening/2018/05/26/6

Jordan


Re: [PATCH 01/24] Add the ability to lock down access to the running kernel image

2018-04-11 Thread Jordan Glover
On April 11, 2018 8:09 PM, Linus Torvalds  wrote:

> On Wed, Apr 11, 2018 at 9:24 AM, David Howells dhowe...@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


Re: [PATCH 01/24] Add the ability to lock down access to the running kernel image

2018-04-11 Thread Jordan Glover
On April 11, 2018 8:09 PM, Linus Torvalds  wrote:

> On Wed, Apr 11, 2018 at 9:24 AM, David Howells dhowe...@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