Re: [PATCH v5 0/6] DCP as trusted keys backend
On Fri, Dec 15, 2023 at 6:07 AM David Gstir wrote:
>
> This is a revival of the previous patch set submitted by Richard Weinberger:
> https://lore.kernel.org/linux-integrity/20210614201620.30451-1-rich...@nod.at/
>
> v4 is here:
> https://lore.kernel.org/keyrings/20231024162024.51260-1-da...@sigma-star.at/
>
> v4 -> v5:
> - Make Kconfig for trust source check scalable as suggested by Jarkko Sakkinen
> - Add Acked-By from Herbert Xu to patch #1 - thanks!
> v3 -> v4:
> - Split changes on MAINTAINERS and documentation into dedicated patches
> - Use more concise wording in commit messages as suggested by Jarkko Sakkinen
> v2 -> v3:
> - Addressed review comments from Jarkko Sakkinen
> v1 -> v2:
> - Revive and rebase to latest version
> - Include review comments from Ahmad Fatoum
>
> The Data CoProcessor (DCP) is an IP core built into many NXP SoCs such
> as i.mx6ull.
>
> Similar to the CAAM engine used in more powerful SoCs, DCP can AES-
> encrypt/decrypt user data using a unique, never-disclosed,
> device-specific key. Unlike CAAM though, it cannot directly wrap and
> unwrap blobs in hardware. As DCP offers only the bare minimum feature
> set and a blob mechanism needs aid from software. A blob in this case
> is a piece of sensitive data (e.g. a key) that is encrypted and
> authenticated using the device-specific key so that unwrapping can only
> be done on the hardware where the blob was wrapped.
>
> This patch series adds a DCP based, trusted-key backend and is similar
> in spirit to the one by Ahmad Fatoum [0] that does the same for CAAM.
> It is of interest for similar use cases as the CAAM patch set, but for
> lower end devices, where CAAM is not available.
>
> Because constructing and parsing the blob has to happen in software,
> we needed to decide on a blob format and chose the following:
>
> struct dcp_blob_fmt {
> __u8 fmt_version;
> __u8 blob_key[AES_KEYSIZE_128];
> __u8 nonce[AES_KEYSIZE_128];
> __le32 payload_len;
> __u8 payload[];
> } __packed;
>
> The `fmt_version` is currently 1.
>
> The encrypted key is stored in the payload area. It is AES-128-GCM
> encrypted using `blob_key` and `nonce`, GCM auth tag is attached at
> the end of the payload (`payload_len` does not include the size of
> the auth tag).
>
> The `blob_key` itself is encrypted in AES-128-ECB mode by DCP using
> the OTP or UNIQUE device key. A new `blob_key` and `nonce` are generated
> randomly, when sealing/exporting the DCP blob.
>
> This patchset was tested with dm-crypt on an i.MX6ULL board.
>
> [0]
> https://lore.kernel.org/keyrings/20220513145705.2080323-1-a.fat...@pengutronix.de/
>
> David Gstir (6):
> crypto: mxs-dcp: Add support for hardware-bound keys
> KEYS: trusted: improve scalability of trust source config
> KEYS: trusted: Introduce NXP DCP-backed trusted keys
> MAINTAINERS: add entry for DCP-based trusted keys
> docs: document DCP-backed trusted keys kernel params
> docs: trusted-encrypted: add DCP as new trust source
>
> .../admin-guide/kernel-parameters.txt | 13 +
> .../security/keys/trusted-encrypted.rst | 85 +
> MAINTAINERS | 9 +
> drivers/crypto/mxs-dcp.c | 104 +-
> include/keys/trusted_dcp.h| 11 +
> include/soc/fsl/dcp.h | 17 +
> security/keys/trusted-keys/Kconfig| 18 +-
> security/keys/trusted-keys/Makefile | 2 +
> security/keys/trusted-keys/trusted_core.c | 6 +-
> security/keys/trusted-keys/trusted_dcp.c | 311 ++
> 10 files changed, 562 insertions(+), 14 deletions(-)
> create mode 100644 include/keys/trusted_dcp.h
> create mode 100644 include/soc/fsl/dcp.h
> create mode 100644 security/keys/trusted-keys/trusted_dcp.c
Jarkko, Mimi, David - if this patchset isn't already in your review
queue, can you take a look at it from a security/keys perspective?
Thanks.
--
paul-moore.com
Re: Login broken with old userspace (was Re: [PATCH v2] selinux: introduce an initial SID for early boot processes)
On Tue, Aug 1, 2023 at 9:24 AM Ondrej Mosnacek wrote: > On Fri, Jul 28, 2023 at 5:12 PM Paul Moore wrote: > > > > On Fri, Jul 28, 2023 at 9:24 AM Christian Göttsche > > wrote: > > > > > > On Fri, 28 Jul 2023 at 15:14, Ondrej Mosnacek wrote: > > > > > > > > On Fri, Jul 28, 2023 at 1:52 PM Stephen Smalley > > > > wrote: > > > > > > > > > > On Fri, Jul 28, 2023 at 7:36 AM Ondrej Mosnacek > > > > > wrote: > > > > > > > > > > > > On Fri, Jul 28, 2023 at 4:12 AM Michael Ellerman > > > > > > wrote: > > > > > > > > > > > > > > Ondrej Mosnacek writes: > > > > > > > > Currently, SELinux doesn't allow distinguishing between kernel > > > > > > > > threads > > > > > > > > and userspace processes that are started before the policy is > > > > > > > > first > > > > > > > > loaded - both get the label corresponding to the kernel SID. > > > > > > > > The only > > > > > > > > way a process that persists from early boot can get a > > > > > > > > meaningful label > > > > > > > > is by doing a voluntary dyntransition or re-executing itself. > > > > > > > > > > > > > > Hi, > > > > > > > > > > > > > > This commit breaks login for me when booting linux-next kernels > > > > > > > with old > > > > > > > userspace, specifically Ubuntu 16.04 on ppc64le. 18.04 is OK. > > > > > > > > > > > > > > The symptom is that login never accepts the root password, it just > > > > > > > always says "Login incorrect". > > > > > > > > > > > > > > Bisect points to this commit. > > > > > > > > > > > > > > Reverting this commit on top of next-20230726, fixes the problem > > > > > > > (ie. login works again). > > > > > > > > > > > > > > Booting with selinux=0 also fixes the problem. > > > > > > > > > > > > > > Is this expected? The change log below suggests backward > > > > > > > compatibility > > > > > > > was considered, is 16.04 just too old? > > > > > > > > > > > > Hi Michael, > > > > > > > > > > > > I can reproduce it on Fedora 38 when I boot with SELINUX=disabled in > > > > > > /etc/selinux/config (+ a kernel including that commit), so it likely > > > > > > isn't caused by the userspace being old. Can you check what you have > > > > > > in /etc/selinux/config (or if it exists at all)? > > > > > > > > > > > > We have deprecated and removed the "runtime disable" functionality > > > > > > in > > > > > > SELinux recently [1], which was used to implement "disabling" > > > > > > SELinux > > > > > > via the /etc/selinux/config file, so now the situation (selinux=0 + > > > > > > SELINUX=disabled in /etc/selinux/config) leads to a state where > > > > > > SELinux is enabled, but no policy is loaded (and no enforcement is > > > > > > done). Such a state mostly behaves as if SElinux was truly disabled > > > > > > (via kernel command line), but there are some subtle differences > > > > > > and I > > > > > > believe we don't officially support it (Paul might clarify). With > > > > > > latest kernels it is recommended to either disable SELinux via the > > > > > > kernel command line (or Kconfig[2]) or to boot it in Enforcing or > > > > > > Permissive mode with a valid/usable policy installed. > > > > > > > > > > > > So I wonder if Ubuntu ships by default with the bad configuration or > > > > > > if it's just a result of using the custom-built linux-next kernel > > > > > > (or > > > > > > some changes on your part). If Ubuntu's stock kernel is configured > > > > > > to > > > > > > boot with SELinux enabled by default, they should also by default > > > > > > ship > > > > > > a usable policy and SELINUX=permissive/enfo
Re: Login broken with old userspace (was Re: [PATCH v2] selinux: introduce an initial SID for early boot processes)
On Fri, Jul 28, 2023 at 9:24 AM Christian Göttsche
wrote:
>
> On Fri, 28 Jul 2023 at 15:14, Ondrej Mosnacek wrote:
> >
> > On Fri, Jul 28, 2023 at 1:52 PM Stephen Smalley
> > wrote:
> > >
> > > On Fri, Jul 28, 2023 at 7:36 AM Ondrej Mosnacek
> > > wrote:
> > > >
> > > > On Fri, Jul 28, 2023 at 4:12 AM Michael Ellerman
> > > > wrote:
> > > > >
> > > > > Ondrej Mosnacek writes:
> > > > > > Currently, SELinux doesn't allow distinguishing between kernel
> > > > > > threads
> > > > > > and userspace processes that are started before the policy is first
> > > > > > loaded - both get the label corresponding to the kernel SID. The
> > > > > > only
> > > > > > way a process that persists from early boot can get a meaningful
> > > > > > label
> > > > > > is by doing a voluntary dyntransition or re-executing itself.
> > > > >
> > > > > Hi,
> > > > >
> > > > > This commit breaks login for me when booting linux-next kernels with
> > > > > old
> > > > > userspace, specifically Ubuntu 16.04 on ppc64le. 18.04 is OK.
> > > > >
> > > > > The symptom is that login never accepts the root password, it just
> > > > > always says "Login incorrect".
> > > > >
> > > > > Bisect points to this commit.
> > > > >
> > > > > Reverting this commit on top of next-20230726, fixes the problem
> > > > > (ie. login works again).
> > > > >
> > > > > Booting with selinux=0 also fixes the problem.
> > > > >
> > > > > Is this expected? The change log below suggests backward compatibility
> > > > > was considered, is 16.04 just too old?
> > > >
> > > > Hi Michael,
> > > >
> > > > I can reproduce it on Fedora 38 when I boot with SELINUX=disabled in
> > > > /etc/selinux/config (+ a kernel including that commit), so it likely
> > > > isn't caused by the userspace being old. Can you check what you have
> > > > in /etc/selinux/config (or if it exists at all)?
> > > >
> > > > We have deprecated and removed the "runtime disable" functionality in
> > > > SELinux recently [1], which was used to implement "disabling" SELinux
> > > > via the /etc/selinux/config file, so now the situation (selinux=0 +
> > > > SELINUX=disabled in /etc/selinux/config) leads to a state where
> > > > SELinux is enabled, but no policy is loaded (and no enforcement is
> > > > done). Such a state mostly behaves as if SElinux was truly disabled
> > > > (via kernel command line), but there are some subtle differences and I
> > > > believe we don't officially support it (Paul might clarify). With
> > > > latest kernels it is recommended to either disable SELinux via the
> > > > kernel command line (or Kconfig[2]) or to boot it in Enforcing or
> > > > Permissive mode with a valid/usable policy installed.
> > > >
> > > > So I wonder if Ubuntu ships by default with the bad configuration or
> > > > if it's just a result of using the custom-built linux-next kernel (or
> > > > some changes on your part). If Ubuntu's stock kernel is configured to
> > > > boot with SELinux enabled by default, they should also by default ship
> > > > a usable policy and SELINUX=permissive/enforcing in
> > > > /etc/selinux/config (or configure the kernel[2] or bootloader to boot
> > > > with SELinux disabled by default). (Although if they ship a pre-[1]
> > > > kernel, they may continue to rely on the runtime disable
> > > > functionality, but it means people will have to be careful when
> > > > booting newer or custom kernels.)
> > > >
> > > > That said, I'd like to get to the bottom of why the commit causes the
> > > > login to fail and fix it somehow. I presume something in PAM chokes on
> > > > the fact that userspace tasks now have "init" instead of "kernel" as
> > > > the pre-policy-load security context, but so far I haven't been able
> > > > to pinpoint the problem. I'll keep digging...
> > > >
> > > > [1]
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f22f9aaf6c3d92ebd5ad9e67acc03afebaaeb289
> > > > [2] via CONFIG_LSM (or CONFIG_SECURITY_SELINUX_BOOTPARAM_VALUE on older
> > > > kernels)
> > >
> > > Prior to selinux userspace commit
> > > 685f4aeeadc0b60f3770404d4f149610d656e3c8 ("libselinux:
> > > is_selinux_enabled(): drop no-policy-loaded test.") libselinux was
> > > checking the result of reading /proc/self/attr/current to see if it
> > > returned the "kernel" string as a means of detecting a system with
> > > SELinux enabled but no policy loaded, and treated that as if SELinux
> > > were disabled. Hence, this does break old userspace. Not sure though
> > > why you'd see the same behavior with modern libselinux.
> >
> > Hm... now I tried booting the stock Fedora kernel (without the early
> > boot initial SID commit) and I got the same failure to login as with
> > the new kernel. So if Ubuntu 16.04 ships with pre-685f4aeeadc0
> > libselinux (quite possible), then it seems that the scenario with
> > terminal login + SELinux enabled + policy not loaded only works with
> > pre-685f4aeeadc0 libselinux and pre-5b0eea835d4e kernel, the other
> >
Re: [RFC PATCH v11 11/29] security: Export security_inode_init_security_anon() for use by KVM
On Tue, Jul 18, 2023 at 7:48 PM Sean Christopherson wrote: > > Signed-off-by: Sean Christopherson > --- > security/security.c | 1 + > 1 file changed, 1 insertion(+) Acked-by: Paul Moore > diff --git a/security/security.c b/security/security.c > index b720424ca37d..7fc78f0f3622 100644 > --- a/security/security.c > +++ b/security/security.c > @@ -1654,6 +1654,7 @@ int security_inode_init_security_anon(struct inode > *inode, > return call_int_hook(inode_init_security_anon, 0, inode, name, > context_inode); > } > +EXPORT_SYMBOL_GPL(security_inode_init_security_anon); > > #ifdef CONFIG_SECURITY_PATH > /** > -- > 2.41.0.255.g8b1d071c50-goog -- paul-moore.com
Re: [PATCH 4/14] audit: avoid missing-prototype warnings
On Wed, May 17, 2023 at 10:51 AM Arnd Bergmann wrote: > On Wed, May 17, 2023, at 16:33, Paul Moore wrote: > > On May 17, 2023 Arnd Bergmann wrote: > > > We probably should move the audit_serial() and auditsc_get_stamp() > > away from the watch/mark/tree functions, but that isn't your problem. > > > > Anyway, this looks okay to me; do you have a problem if I merge this > > via the audit/next branch or were you hoping to have this go in > > through a different tree? > > Merging it through your tree is probably best, Andrew can either > pick the ones that nobody else took, or I can resend the rest. Easy enough, merged to audit/next, thanks. -- paul-moore.com
Re: [PATCH 4/14] audit: avoid missing-prototype warnings
On May 17, 2023 Arnd Bergmann wrote:
>
> Building with 'make W=1' reveals two function definitions without
> a previous prototype in the audit code:
>
> lib/compat_audit.c:32:5: error: no previous prototype for
> 'audit_classify_compat_syscall' [-Werror=missing-prototypes]
> kernel/audit.c:1813:14: error: no previous prototype for 'audit_serial'
> [-Werror=missing-prototypes]
>
> The first one needs a declaration from linux/audit.h but cannot
> include that header without causing conflicting (compat) syscall number
> definitions, so move the it into linux/audit_arch.h.
>
> The second one is declared conditionally based on CONFIG_AUDITSYSCALL
> but needed as a local function even when that option is disabled, so
> move the declaration out of the #ifdef block.
>
> Signed-off-by: Arnd Bergmann
> ---
> include/linux/audit.h | 2 --
> include/linux/audit_arch.h | 2 ++
> kernel/audit.h | 2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index 31086a72e32a..6a3a9e122bb5 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -130,8 +130,6 @@ extern unsigned compat_dir_class[];
> extern unsigned compat_chattr_class[];
> extern unsigned compat_signal_class[];
>
> -extern int audit_classify_compat_syscall(int abi, unsigned syscall);
> -
> /* audit_names->type values */
> #define AUDIT_TYPE_UNKNOWN 0 /* we don't know yet */
> #define AUDIT_TYPE_NORMAL 1 /* a "normal" audit record */
> diff --git a/include/linux/audit_arch.h b/include/linux/audit_arch.h
> index 8fdb1afe251a..0e34d673ef17 100644
> --- a/include/linux/audit_arch.h
> +++ b/include/linux/audit_arch.h
> @@ -21,4 +21,6 @@ enum auditsc_class_t {
> AUDITSC_NVALS /* count */
> };
>
> +extern int audit_classify_compat_syscall(int abi, unsigned syscall);
> +
> #endif
> diff --git a/kernel/audit.h b/kernel/audit.h
> index c57b008b9914..94738bce40b2 100644
> --- a/kernel/audit.h
> +++ b/kernel/audit.h
> @@ -259,8 +259,8 @@ extern struct tty_struct *audit_get_tty(void);
> extern void audit_put_tty(struct tty_struct *tty);
>
> /* audit watch/mark/tree functions */
> -#ifdef CONFIG_AUDITSYSCALL
> extern unsigned int audit_serial(void);
> +#ifdef CONFIG_AUDITSYSCALL
> extern int auditsc_get_stamp(struct audit_context *ctx,
> struct timespec64 *t, unsigned int *serial);
We probably should move the audit_serial() and auditsc_get_stamp()
away from the watch/mark/tree functions, but that isn't your problem.
Anyway, this looks okay to me; do you have a problem if I merge this
via the audit/next branch or were you hoping to have this go in
through a different tree?
--
paul-moore.com
Re: [PATCH v2 2/2] powerpc/rtas: block error injection when locked down
On Mon, Sep 26, 2022 at 9:18 AM Nathan Lynch wrote: > > The error injection facility on pseries VMs allows corruption of > arbitrary guest memory, potentially enabling a sufficiently privileged > user to disable lockdown or perform other modifications of the running > kernel via the rtas syscall. > > Block the PAPR error injection facility from being opened or called > when locked down. > > Signed-off-by: Nathan Lynch > --- > arch/powerpc/kernel/rtas.c | 25 - > include/linux/security.h | 1 + > security/security.c| 1 + > 3 files changed, 26 insertions(+), 1 deletion(-) The lockdown changes are trivial, but they look fine to me. Acked-by: Paul Moore (LSM) -- paul-moore.com
Re: [PATCH v2 1/2] powerpc/pseries: block untrusted device tree changes when locked down
On Mon, Sep 26, 2022 at 9:17 AM Nathan Lynch wrote:
>
> The /proc/powerpc/ofdt interface allows the root user to freely alter
> the in-kernel device tree, enabling arbitrary physical address writes
> via drivers that could bind to malicious device nodes, thus making it
> possible to disable lockdown.
>
> Historically this interface has been used on the pseries platform to
> facilitate the runtime addition and removal of processor, memory, and
> device resources (aka Dynamic Logical Partitioning or DLPAR). Years
> ago, the processor and memory use cases were migrated to designs that
> happen to be lockdown-friendly: device tree updates are communicated
> directly to the kernel from firmware without passing through untrusted
> user space. I/O device DLPAR via the "drmgr" command in powerpc-utils
> remains the sole legitimate user of /proc/powerpc/ofdt, but it is
> already broken in lockdown since it uses /dev/mem to allocate argument
> buffers for the rtas syscall. So only illegitimate uses of the
> interface should see a behavior change when running on a locked down
> kernel.
>
> Signed-off-by: Nathan Lynch
> ---
> arch/powerpc/platforms/pseries/reconfig.c | 5 +
> include/linux/security.h | 1 +
> security/security.c | 1 +
> 3 files changed, 7 insertions(+)
Thanks for moving the definitions.
Acked-by: Paul Moore (LSM)
> diff --git a/arch/powerpc/platforms/pseries/reconfig.c
> b/arch/powerpc/platforms/pseries/reconfig.c
> index cad7a0c93117..599bd2c78514 100644
> --- a/arch/powerpc/platforms/pseries/reconfig.c
> +++ b/arch/powerpc/platforms/pseries/reconfig.c
> @@ -10,6 +10,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
>
> @@ -361,6 +362,10 @@ static ssize_t ofdt_write(struct file *file, const char
> __user *buf, size_t coun
> char *kbuf;
> char *tmp;
>
> + rv = security_locked_down(LOCKDOWN_DEVICE_TREE);
> + if (rv)
> + return rv;
> +
> kbuf = memdup_user_nul(buf, count);
> if (IS_ERR(kbuf))
> return PTR_ERR(kbuf);
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 7bd0c490703d..39e7c0e403d9 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -114,6 +114,7 @@ enum lockdown_reason {
> LOCKDOWN_IOPORT,
> LOCKDOWN_MSR,
> LOCKDOWN_ACPI_TABLES,
> + LOCKDOWN_DEVICE_TREE,
> LOCKDOWN_PCMCIA_CIS,
> LOCKDOWN_TIOCSSERIAL,
> LOCKDOWN_MODULE_PARAMETERS,
> diff --git a/security/security.c b/security/security.c
> index 4b95de24bc8d..51bf66d4f472 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -52,6 +52,7 @@ const char *const
> lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
> [LOCKDOWN_IOPORT] = "raw io port access",
> [LOCKDOWN_MSR] = "raw MSR access",
> [LOCKDOWN_ACPI_TABLES] = "modifying ACPI tables",
> + [LOCKDOWN_DEVICE_TREE] = "modifying device tree contents",
> [LOCKDOWN_PCMCIA_CIS] = "direct PCMCIA CIS storage",
> [LOCKDOWN_TIOCSSERIAL] = "reconfiguration of serial port IO",
> [LOCKDOWN_MODULE_PARAMETERS] = "unsafe module parameters",
> --
> 2.37.3
>
--
paul-moore.com
Re: [PATCH 2/2] powerpc/rtas: block error injection when locked down
On Fri, Sep 23, 2022 at 11:40 AM Nathan Lynch wrote:
> Michael Ellerman writes:
> > Paul Moore writes:
> >> On Thu, Sep 22, 2022 at 3:38 PM Nathan Lynch wrote:
> >>>
> >>> The error injection facility on pseries VMs allows corruption of
> >>> arbitrary guest memory, potentially enabling a sufficiently privileged
> >>> user to disable lockdown or perform other modifications of the running
> >>> kernel via the rtas syscall.
> >>>
> >>> Block the PAPR error injection facility from being opened or called
> >>> when locked down.
> >>>
> >>> Signed-off-by: Nathan Lynch
> >>> ---
> >>> arch/powerpc/kernel/rtas.c | 25 -
> >>> include/linux/security.h | 1 +
> >>> security/security.c| 1 +
> >>> 3 files changed, 26 insertions(+), 1 deletion(-)
> >>
> >> ...
> >>
> >>> diff --git a/include/linux/security.h b/include/linux/security.h
> >>> index 1ca8dbacd3cc..b5d5138ae66a 100644
> >>> --- a/include/linux/security.h
> >>> +++ b/include/linux/security.h
> >>> @@ -123,6 +123,7 @@ enum lockdown_reason {
> >>> LOCKDOWN_BPF_WRITE_USER,
> >>> LOCKDOWN_DBG_WRITE_KERNEL,
> >>> LOCKDOWN_DEVICE_TREE,
> >>> + LOCKDOWN_RTAS_ERROR_INJECTION,
> >>
> >> With the understanding that I've never heard of RTAS until now, are
> >> there any other RTAS events that would require a lockdown reason? As
> >> a follow up, is it important to distinguish between different RTAS
> >> lockdown reasons?
>
> 1. Not to my current knowledge.
> 2. Yes, I think so, see below.
>
> >>
> >> I'm trying to determine if we can just call it LOCKDOWN_RTAS.
> >
> > Yes I think we should.
> >
> > Currently it only locks down the error injection calls, not all of RTAS.
> >
> > But firmware can/will add new RTAS calls in future, so it's always
> > possible something will need to be added to the list of things that need
> > to be blocked during lockdown.
> >
> > So I think calling it LOCKDOWN_RTAS would be more general and future
> > proof, and can be read to mean "lockdown the parts of RTAS that need
> > to be locked down".
>
> RTAS provides callable interfaces for a broad variety of functions,
> including device configuration, halt/reboot/suspend, CPU online/offline,
> NVRAM access, firmware upgrade, platform diagnostic data retrieval, and
> others.
>
> Currently I don't know of other RTAS-provided functions that should be
> restricted. But if we were to add more, then -- in answer to Paul -- yes
> I think it would be important to have distinct reasons and
> messages. Taking the point of view of someone diagnosing lockdown denial
> messages and relating them to kernel code and user space activity, I
> would rather we err toward specificity.
As I said before, RTAS is a great mystery to me, if it can be extended
in the future then having a targeted lockdown name makes perfect
sense.
> So a single RTAS catch-all lockdown reason doesn't appeal to me, but
> lockdown reasons and messages aren't ABI (right?) ...
Correct. Or at least that is my understanding, but there have been
some odd rulings on lockdown in the past so my advice would be to make
*very* sure you get this right the first time. From what you and
Michael have said, it seems like a function specific name is the way
to go here, and based on your explanations of the situation it seems
like putting this in the integrity bin is the right way to go.
--
paul-moore.com
Re: [PATCH 2/2] powerpc/rtas: block error injection when locked down
On Thu, Sep 22, 2022 at 3:38 PM Nathan Lynch wrote:
>
> The error injection facility on pseries VMs allows corruption of
> arbitrary guest memory, potentially enabling a sufficiently privileged
> user to disable lockdown or perform other modifications of the running
> kernel via the rtas syscall.
>
> Block the PAPR error injection facility from being opened or called
> when locked down.
>
> Signed-off-by: Nathan Lynch
> ---
> arch/powerpc/kernel/rtas.c | 25 -
> include/linux/security.h | 1 +
> security/security.c| 1 +
> 3 files changed, 26 insertions(+), 1 deletion(-)
...
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 1ca8dbacd3cc..b5d5138ae66a 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -123,6 +123,7 @@ enum lockdown_reason {
> LOCKDOWN_BPF_WRITE_USER,
> LOCKDOWN_DBG_WRITE_KERNEL,
> LOCKDOWN_DEVICE_TREE,
> + LOCKDOWN_RTAS_ERROR_INJECTION,
With the understanding that I've never heard of RTAS until now, are
there any other RTAS events that would require a lockdown reason? As
a follow up, is it important to distinguish between different RTAS
lockdown reasons?
I'm trying to determine if we can just call it LOCKDOWN_RTAS.
> LOCKDOWN_INTEGRITY_MAX,
> LOCKDOWN_KCORE,
> LOCKDOWN_KPROBES,
> diff --git a/security/security.c b/security/security.c
> index 2863fc31eec6..6518b239ada2 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -61,6 +61,7 @@ const char *const
> lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
> [LOCKDOWN_BPF_WRITE_USER] = "use of bpf to write user RAM",
> [LOCKDOWN_DBG_WRITE_KERNEL] = "use of kgdb/kdb to write kernel RAM",
> [LOCKDOWN_DEVICE_TREE] = "modifying device tree contents",
> + [LOCKDOWN_RTAS_ERROR_INJECTION] = "RTAS error injection",
See above.
> [LOCKDOWN_INTEGRITY_MAX] = "integrity",
> [LOCKDOWN_KCORE] = "/proc/kcore access",
> [LOCKDOWN_KPROBES] = "use of kprobes",
> --
> 2.37.3
--
paul-moore.com
Re: [PATCH 1/2] powerpc/pseries: block untrusted device tree changes when locked down
On Thu, Sep 22, 2022 at 3:38 PM Nathan Lynch wrote:
>
> The /proc/powerpc/ofdt interface allows the root user to freely alter
> the in-kernel device tree, enabling arbitrary physical address writes
> via drivers that could bind to malicious device nodes, thus making it
> possible to disable lockdown.
>
> Historically this interface has been used on the pseries platform to
> facilitate the runtime addition and removal of processor, memory, and
> device resources (aka Dynamic Logical Partitioning or DLPAR). Years
> ago, the processor and memory use cases were migrated to designs that
> happen to be lockdown-friendly: device tree updates are communicated
> directly to the kernel from firmware without passing through untrusted
> user space. I/O device DLPAR via the "drmgr" command in powerpc-utils
> remains the sole legitimate user of /proc/powerpc/ofdt, but it is
> already broken in lockdown since it uses /dev/mem to allocate argument
> buffers for the rtas syscall. So only illegitimate uses of the
> interface should see a behavior change when running on a locked down
> kernel.
>
> Signed-off-by: Nathan Lynch
> ---
> arch/powerpc/platforms/pseries/reconfig.c | 5 +
> include/linux/security.h | 1 +
> security/security.c | 1 +
> 3 files changed, 7 insertions(+)
A couple of small nits below, but in general this seems reasonable.
However, as we are currently at -rc6 I would like us to wait to merge
this until after the upcoming merge window closes (I don't like
merging new functionality into -next at -rc6).
https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/lsm.git/tree/README.md
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 7bd0c490703d..1ca8dbacd3cc 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -122,6 +122,7 @@ enum lockdown_reason {
> LOCKDOWN_XMON_WR,
> LOCKDOWN_BPF_WRITE_USER,
> LOCKDOWN_DBG_WRITE_KERNEL,
> + LOCKDOWN_DEVICE_TREE,
I would suggest moving LOCKDOWN_DEVICE_TREE to be next to
LOCKDOWN_ACPI_TABLES. It's not a hard requirement, but it seems like
a nice idea to group similar things when we can.
> LOCKDOWN_INTEGRITY_MAX,
> LOCKDOWN_KCORE,
> LOCKDOWN_KPROBES,
> diff --git a/security/security.c b/security/security.c
> index 4b95de24bc8d..2863fc31eec6 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -60,6 +60,7 @@ const char *const
> lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
> [LOCKDOWN_XMON_WR] = "xmon write access",
> [LOCKDOWN_BPF_WRITE_USER] = "use of bpf to write user RAM",
> [LOCKDOWN_DBG_WRITE_KERNEL] = "use of kgdb/kdb to write kernel RAM",
> + [LOCKDOWN_DEVICE_TREE] = "modifying device tree contents",
Might as well move this one too.
> [LOCKDOWN_INTEGRITY_MAX] = "integrity",
> [LOCKDOWN_KCORE] = "/proc/kcore access",
> [LOCKDOWN_KPROBES] = "use of kprobes",
> --
> 2.37.3
--
paul-moore.com
Re: linux-next: manual merge of the audit tree with the powerpc tree
On Fri, Dec 17, 2021 at 9:11 AM Christophe Leroy wrote: > Le 17/12/2021 à 00:04, Paul Moore a écrit : > > On Thu, Dec 16, 2021 at 4:08 AM Christophe Leroy > > wrote: > >> Thanks Cédric, I've now been able to install debian PPC32 port of DEBIAN > >> 11 on QEMU and run the tests. > >> > >> I followed instructions in file README.md provided in the test suite. > >> I also modified tests/Makefile to force MODE := 32 > >> > >> I've got a lot of failures, am I missing some options in the kernel or > >> something ? > >> > >> Running as userroot > >> with context root::: > >> on system > > > > While SELinux is not required for audit, I don't think I've ever run > > it on system without SELinux. In theory the audit-testsuite shouldn't > > rely on SELinux being present (other than the SELinux specific tests > > of course), but I'm not confident enough to say that the test suite > > will run without problem without SELinux. > > > > If it isn't too difficult, I would suggest enabling SELinux in your > > kernel build and ensuring the necessary userspace, policy, etc. is > > installed. You don't need to worry about getting it all running > > correctly; the audit-testsuite should pass with SELinux in permissive > > mode. > > > > If you're still seeing all these failures after trying that let us know. > > > > Still the same it seems: > > Running as userroot > with context unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 > on system > > # Test 3 got: "256" (backlog_wait_time_actual_reset/test at line 151) > # Expected: "0" > # backlog_wait_time_actual_reset/test line 151 is: ok( $result, 0 ); > # Was an event found? My apologies, this thread was lost in the end-of-year holidays. At this point, and with that many failures, I think you'll need to spend some time debugging the test failures to see what is wrong. I don't have a PPC32 system/VM and I don't have the time right now to build up a PPC32 test environment. -- paul moore www.paul-moore.com
Re: linux-next: manual merge of the audit tree with the powerpc tree
On Thu, Dec 16, 2021 at 4:08 AM Christophe Leroy wrote: > Thanks Cédric, I've now been able to install debian PPC32 port of DEBIAN > 11 on QEMU and run the tests. > > I followed instructions in file README.md provided in the test suite. > I also modified tests/Makefile to force MODE := 32 > > I've got a lot of failures, am I missing some options in the kernel or > something ? > > Running as userroot > with context root::: > on system While SELinux is not required for audit, I don't think I've ever run it on system without SELinux. In theory the audit-testsuite shouldn't rely on SELinux being present (other than the SELinux specific tests of course), but I'm not confident enough to say that the test suite will run without problem without SELinux. If it isn't too difficult, I would suggest enabling SELinux in your kernel build and ensuring the necessary userspace, policy, etc. is installed. You don't need to worry about getting it all running correctly; the audit-testsuite should pass with SELinux in permissive mode. If you're still seeing all these failures after trying that let us know. > # Test 3 got: "256" (backlog_wait_time_actual_reset/test at line 151) > # Expected: "0" > # backlog_wait_time_actual_reset/test line 151 is: ok( $result, 0 ); > # Was an event found? ... -- paul moore www.paul-moore.com
Re: linux-next: manual merge of the audit tree with the powerpc tree
On Tue, Dec 14, 2021 at 12:59 PM Christophe Leroy wrote: > Hello Paul, > > I've been trying to setup your test suite on my powerpc board but it's > based on Perl and on a lot of optional Perl packages. I was able to add > them one by one until some of them require some .so libraries > (Pathtools-Cwd), and it seems nothing is made to allow cross building > those libraries. > > Do you have another test suite based on C and not perl ? > > If not, what can I do, do you know how I can cross compile those Perl > packages for PPC32 ? Is there no Linux distribution that supports PPC32? I would think that would be the easiest path forward, but you're the PPC32 expert - not me - so I'll assume you already tried that or it didn't work for other reasons. I'm also not a Perl expert, but it looks like PathTools is part of the core Perl5 release, have you tried that? https://github.com/Perl/perl5/tree/blead/dist/PathTools Finally, no, our only really maintained test suite is the Perl based one; there have been other efforts over the years but they were never properly supported and fell out of use (and applicability). At some point you/someone was able to run the test suite, why isn't that working now? Or was it a different powerpc ABI? -- paul moore www.paul-moore.com
Re: [PATCH v2 RESEND] powerpc/audit: Convert powerpc to AUDIT_ARCH_COMPAT_GENERIC
On Thu, Nov 4, 2021 at 2:20 AM Michael Ellerman wrote:
> Konstantin Ryabitsev writes:
> > On Wed, Nov 03, 2021 at 10:18:57AM +1100, Michael Ellerman wrote:
> >> It's not in next, that notification is from the b4 thanks script, which
> >> didn't notice that the commit has since been reverted.
> >
> > Yeah... I'm not sure how to catch that, but I'm open to suggestions.
>
> I think that's probably the first time I've had a commit and a revert of
> the commit in the same batch of thanks mails.
>
> And the notification is not wrong, the commit was applied with that SHA,
> it is in the tree.
>
> So I'm not sure it's very common to have a commit & a revert in the tree
> at the same time.
I know it is not common for the SELinux and audit trees. I guess
every tree/maintainer is different, but I'm probably more conservative
than most when it comes to merging patches so it's pretty rare that we
need to revert things in those trees.
> On the other hand being able to generate a mail for an arbitrary revert
> would be helpful, ie. independent of any thanks state.
>
> eg, picking a random commit from the past:
>
> e95ad5f21693 ("powerpc/head_check: Fix shellcheck errors")
>
> If I revert that in my tree today, it'd be cool if I could run something
> that would detect the revert, backtrack to the reverted commit, extract
> the message-id from the Link: tag, and generate a reply to the original
> submission noting that it's now been reverted.
Even if it isn't practical to do the find-the-original-commit logic,
simply getting an email that says "FYI, this revert has been merged
into this tree/branch" would be helpful. Although I guess that would
require either the revert having the right metadata, e.g. "Cc:", or
that prior mentioned logic to find the original commit so the proper
To/CC lines could be generated.
--
paul moore
www.paul-moore.com
Re: [PATCH v2 RESEND] powerpc/audit: Convert powerpc to AUDIT_ARCH_COMPAT_GENERIC
On Tue, Nov 2, 2021 at 7:19 PM Michael Ellerman wrote:
> Paul Moore writes:
> > On Tue, Nov 2, 2021 at 7:38 AM Michael Ellerman
> > wrote:
> >>
> >> On Tue, 24 Aug 2021 13:36:13 + (UTC), Christophe Leroy wrote:
> >> > Commit e65e1fc2d24b ("[PATCH] syscall class hookup for all normal
> >> > targets") added generic support for AUDIT but that didn't include
> >> > support for bi-arch like powerpc.
> >> >
> >> > Commit 4b58841149dc ("audit: Add generic compat syscall support")
> >> > added generic support for bi-arch.
> >> >
> >> > [...]
> >>
> >> Applied to powerpc/next.
> >>
> >> [1/1] powerpc/audit: Convert powerpc to AUDIT_ARCH_COMPAT_GENERIC
> >>
> >> https://git.kernel.org/powerpc/c/566af8cda399c088763d07464463dc871c943b54
> >
> > Did the test failure discussed earlier in this thread ever get
> > resolved? If not, this really shouldn't be in linux-next IMO.
>
> It's not in next, that notification is from the b4 thanks script, which
> didn't notice that the commit has since been reverted.
Okay, thanks for the clarification. I know we had already talked
about this so I was a little caught off guard when I saw this mail :)
--
paul moore
www.paul-moore.com
Re: [PATCH v2 RESEND] powerpc/audit: Convert powerpc to AUDIT_ARCH_COMPAT_GENERIC
On Tue, Nov 2, 2021 at 7:38 AM Michael Ellerman
wrote:
>
> On Tue, 24 Aug 2021 13:36:13 + (UTC), Christophe Leroy wrote:
> > Commit e65e1fc2d24b ("[PATCH] syscall class hookup for all normal
> > targets") added generic support for AUDIT but that didn't include
> > support for bi-arch like powerpc.
> >
> > Commit 4b58841149dc ("audit: Add generic compat syscall support")
> > added generic support for bi-arch.
> >
> > [...]
>
> Applied to powerpc/next.
>
> [1/1] powerpc/audit: Convert powerpc to AUDIT_ARCH_COMPAT_GENERIC
>
> https://git.kernel.org/powerpc/c/566af8cda399c088763d07464463dc871c943b54
Did the test failure discussed earlier in this thread ever get
resolved? If not, this really shouldn't be in linux-next IMO.
--
paul moore
www.paul-moore.com
Re: linux-next: manual merge of the audit tree with the powerpc tree
On Wed, Oct 27, 2021 at 7:41 AM Christophe Leroy
wrote:
> Le 27/10/2021 à 13:29, Michael Ellerman a écrit :
> > Paul Moore writes:
> >> On Tue, Oct 26, 2021 at 6:55 AM Michael Ellerman
> >> wrote:
> >>> Stephen Rothwell writes:
> >>>> Hi all,
> >>>>
> >>>> Today's linux-next merge of the audit tree got conflicts in:
> >>>>
> >>>>arch/powerpc/kernel/audit.c
> >>>>arch/powerpc/kernel/compat_audit.c
> >>>>
> >>>> between commit:
> >>>>
> >>>>566af8cda399 ("powerpc/audit: Convert powerpc to
> >>>> AUDIT_ARCH_COMPAT_GENERIC")
> >>>>
> >>>> from the powerpc tree and commits:
> >>>>
> >>>>42f355ef59a2 ("audit: replace magic audit syscall class numbers with
> >>>> macros")
> >>>>1c30e3af8a79 ("audit: add support for the openat2 syscall")
> >>>>
> >>>> from the audit tree.
> >>>
> >>> Thanks.
> >>>
> >>> I guess this is OK, unless the audit folks disagree. I could revert the
> >>> powerpc commit and try it again later.
> >>>
> >>> If I don't hear anything I'll leave it as-is.
> >>
> >> Hi Michael,
> >>
> >> Last I recall from the powerpc/audit thread there were still some
> >> issues with audit working properly in your testing, has that been
> >> resolved?
> >
> > No.
> >
> > There's one test failure both before and after the conversion to use the
> > generic code.
> >
> >> If nothing else, -rc7 seems a bit late for this to hit -next for me to
> >> feel comfortable about this.
> >
> > OK. I'll revert the patch in my tree.
>
> But it's been in the pipe since end of August and no one reported any
> issue other issue than the pre-existing one, so what's the new issue
> that prevents us to merge it two monthes later, and how do we walk
> forward then ?
We work to resolve the test failure, it's that simple. I haven't seen
the failure so I haven't been much help to do any sort of root cause
digging on the problem, it would be helpful if those who are seeing
the problem could dig into the failure and report back on what they
find. That is what has been missing and why I never ACK'd or merged
the powerpc audit code.
--
paul moore
www.paul-moore.com
Re: linux-next: manual merge of the audit tree with the powerpc tree
On Tue, Oct 26, 2021 at 6:55 AM Michael Ellerman wrote:
>
> Stephen Rothwell writes:
> > Hi all,
> >
> > Today's linux-next merge of the audit tree got conflicts in:
> >
> > arch/powerpc/kernel/audit.c
> > arch/powerpc/kernel/compat_audit.c
> >
> > between commit:
> >
> > 566af8cda399 ("powerpc/audit: Convert powerpc to
> > AUDIT_ARCH_COMPAT_GENERIC")
> >
> > from the powerpc tree and commits:
> >
> > 42f355ef59a2 ("audit: replace magic audit syscall class numbers with
> > macros")
> > 1c30e3af8a79 ("audit: add support for the openat2 syscall")
> >
> > from the audit tree.
>
> Thanks.
>
> I guess this is OK, unless the audit folks disagree. I could revert the
> powerpc commit and try it again later.
>
> If I don't hear anything I'll leave it as-is.
Hi Michael,
Last I recall from the powerpc/audit thread there were still some
issues with audit working properly in your testing, has that been
resolved? If nothing else, -rc7 seems a bit late for this to hit
-next for me to feel comfortable about this.
--
paul moore
www.paul-moore.com
Re: [PATCH v4] lockdown,selinux: fix wrong subject in some SELinux lockdown checks
On Wed, Sep 15, 2021 at 10:59 PM Paul Moore wrote:
>
> On Mon, Sep 13, 2021 at 5:05 PM Paul Moore wrote:
> >
> > On Mon, Sep 13, 2021 at 10:02 AM Ondrej Mosnacek
> > wrote:
> > >
> > > Commit 59438b46471a ("security,lockdown,selinux: implement SELinux
> > > lockdown") added an implementation of the locked_down LSM hook to
> > > SELinux, with the aim to restrict which domains are allowed to perform
> > > operations that would breach lockdown.
> > >
> > > However, in several places the security_locked_down() hook is called in
> > > situations where the current task isn't doing any action that would
> > > directly breach lockdown, leading to SELinux checks that are basically
> > > bogus.
> > >
> > > To fix this, add an explicit struct cred pointer argument to
> > > security_lockdown() and define NULL as a special value to pass instead
> > > of current_cred() in such situations. LSMs that take the subject
> > > credentials into account can then fall back to some default or ignore
> > > such calls altogether. In the SELinux lockdown hook implementation, use
> > > SECINITSID_KERNEL in case the cred argument is NULL.
> > >
> > > Most of the callers are updated to pass current_cred() as the cred
> > > pointer, thus maintaining the same behavior. The following callers are
> > > modified to pass NULL as the cred pointer instead:
> > > 1. arch/powerpc/xmon/xmon.c
> > > Seems to be some interactive debugging facility. It appears that
> > > the lockdown hook is called from interrupt context here, so it
> > > should be more appropriate to request a global lockdown decision.
> > > 2. fs/tracefs/inode.c:tracefs_create_file()
> > > Here the call is used to prevent creating new tracefs entries when
> > > the kernel is locked down. Assumes that locking down is one-way -
> > > i.e. if the hook returns non-zero once, it will never return zero
> > > again, thus no point in creating these files. Also, the hook is
> > > often called by a module's init function when it is loaded by
> > > userspace, where it doesn't make much sense to do a check against
> > > the current task's creds, since the task itself doesn't actually
> > > use the tracing functionality (i.e. doesn't breach lockdown), just
> > > indirectly makes some new tracepoints available to whoever is
> > > authorized to use them.
> > > 3. net/xfrm/xfrm_user.c:copy_to_user_*()
> > > Here a cryptographic secret is redacted based on the value returned
> > > from the hook. There are two possible actions that may lead here:
> > > a) A netlink message XFRM_MSG_GETSA with NLM_F_DUMP set - here the
> > > task context is relevant, since the dumped data is sent back to
> > > the current task.
> > > b) When adding/deleting/updating an SA via XFRM_MSG_xxxSA, the
> > > dumped SA is broadcasted to tasks subscribed to XFRM events -
> > > here the current task context is not relevant as it doesn't
> > > represent the tasks that could potentially see the secret.
> > > It doesn't seem worth it to try to keep using the current task's
> > > context in the a) case, since the eventual data leak can be
> > > circumvented anyway via b), plus there is no way for the task to
> > > indicate that it doesn't care about the actual key value, so the
> > > check could generate a lot of "false alert" denials with SELinux.
> > > Thus, let's pass NULL instead of current_cred() here faute de
> > > mieux.
> > >
> > > Improvements-suggested-by: Casey Schaufler
> > > Improvements-suggested-by: Paul Moore
> > > Fixes: 59438b46471a ("security,lockdown,selinux: implement SELinux
> > > lockdown")
> > > Acked-by: Dan Williams [cxl]
> > > Acked-by: Steffen Klassert [xfrm]
> > > Signed-off-by: Ondrej Mosnacek
> > > ---
> > >
> > > v4:
> > > - rebase on top of TODO
> > > - fix rebase conflicts:
> > > * drivers/cxl/pci.c
> > > - trivial: the lockdown reason was corrected in mainline
> > > * kernel/bpf/helpers.c, kernel/trace/bpf_trace.c
> > > - trivial: LOCKDOWN_BPF_READ was renamed to LOCKDOWN_BPF_READ_KERNEL
> > > in mainline
> > > * kernel/power/hibernate.c
> > > - trivial: !secretmem_active() was added to the conditio
Re: [PATCH v4] lockdown,selinux: fix wrong subject in some SELinux lockdown checks
On Mon, Sep 13, 2021 at 5:05 PM Paul Moore wrote:
>
> On Mon, Sep 13, 2021 at 10:02 AM Ondrej Mosnacek wrote:
> >
> > Commit 59438b46471a ("security,lockdown,selinux: implement SELinux
> > lockdown") added an implementation of the locked_down LSM hook to
> > SELinux, with the aim to restrict which domains are allowed to perform
> > operations that would breach lockdown.
> >
> > However, in several places the security_locked_down() hook is called in
> > situations where the current task isn't doing any action that would
> > directly breach lockdown, leading to SELinux checks that are basically
> > bogus.
> >
> > To fix this, add an explicit struct cred pointer argument to
> > security_lockdown() and define NULL as a special value to pass instead
> > of current_cred() in such situations. LSMs that take the subject
> > credentials into account can then fall back to some default or ignore
> > such calls altogether. In the SELinux lockdown hook implementation, use
> > SECINITSID_KERNEL in case the cred argument is NULL.
> >
> > Most of the callers are updated to pass current_cred() as the cred
> > pointer, thus maintaining the same behavior. The following callers are
> > modified to pass NULL as the cred pointer instead:
> > 1. arch/powerpc/xmon/xmon.c
> > Seems to be some interactive debugging facility. It appears that
> > the lockdown hook is called from interrupt context here, so it
> > should be more appropriate to request a global lockdown decision.
> > 2. fs/tracefs/inode.c:tracefs_create_file()
> > Here the call is used to prevent creating new tracefs entries when
> > the kernel is locked down. Assumes that locking down is one-way -
> > i.e. if the hook returns non-zero once, it will never return zero
> > again, thus no point in creating these files. Also, the hook is
> > often called by a module's init function when it is loaded by
> > userspace, where it doesn't make much sense to do a check against
> > the current task's creds, since the task itself doesn't actually
> > use the tracing functionality (i.e. doesn't breach lockdown), just
> > indirectly makes some new tracepoints available to whoever is
> > authorized to use them.
> > 3. net/xfrm/xfrm_user.c:copy_to_user_*()
> > Here a cryptographic secret is redacted based on the value returned
> > from the hook. There are two possible actions that may lead here:
> > a) A netlink message XFRM_MSG_GETSA with NLM_F_DUMP set - here the
> > task context is relevant, since the dumped data is sent back to
> > the current task.
> > b) When adding/deleting/updating an SA via XFRM_MSG_xxxSA, the
> > dumped SA is broadcasted to tasks subscribed to XFRM events -
> > here the current task context is not relevant as it doesn't
> > represent the tasks that could potentially see the secret.
> > It doesn't seem worth it to try to keep using the current task's
> > context in the a) case, since the eventual data leak can be
> > circumvented anyway via b), plus there is no way for the task to
> > indicate that it doesn't care about the actual key value, so the
> > check could generate a lot of "false alert" denials with SELinux.
> > Thus, let's pass NULL instead of current_cred() here faute de
> > mieux.
> >
> > Improvements-suggested-by: Casey Schaufler
> > Improvements-suggested-by: Paul Moore
> > Fixes: 59438b46471a ("security,lockdown,selinux: implement SELinux
> > lockdown")
> > Acked-by: Dan Williams [cxl]
> > Acked-by: Steffen Klassert [xfrm]
> > Signed-off-by: Ondrej Mosnacek
> > ---
> >
> > v4:
> > - rebase on top of TODO
> > - fix rebase conflicts:
> > * drivers/cxl/pci.c
> > - trivial: the lockdown reason was corrected in mainline
> > * kernel/bpf/helpers.c, kernel/trace/bpf_trace.c
> > - trivial: LOCKDOWN_BPF_READ was renamed to LOCKDOWN_BPF_READ_KERNEL
> > in mainline
> > * kernel/power/hibernate.c
> > - trivial: !secretmem_active() was added to the condition in
> > hibernation_available()
> > - cover new security_locked_down() call in kernel/bpf/helpers.c
> > (LOCKDOWN_BPF_WRITE_USER in BPF_FUNC_probe_write_user case)
> >
> > v3:
> > https://lore.kernel.org/lkml/20210616085118.1141101-1-omosn...@redhat.com/
> > - add the cred argument to security_locked_down() and adapt all callers
> > - keep using current_cred() in BPF, as the hook
Re: [PATCH v4] lockdown,selinux: fix wrong subject in some SELinux lockdown checks
On Mon, Sep 13, 2021 at 10:02 AM Ondrej Mosnacek wrote:
>
> Commit 59438b46471a ("security,lockdown,selinux: implement SELinux
> lockdown") added an implementation of the locked_down LSM hook to
> SELinux, with the aim to restrict which domains are allowed to perform
> operations that would breach lockdown.
>
> However, in several places the security_locked_down() hook is called in
> situations where the current task isn't doing any action that would
> directly breach lockdown, leading to SELinux checks that are basically
> bogus.
>
> To fix this, add an explicit struct cred pointer argument to
> security_lockdown() and define NULL as a special value to pass instead
> of current_cred() in such situations. LSMs that take the subject
> credentials into account can then fall back to some default or ignore
> such calls altogether. In the SELinux lockdown hook implementation, use
> SECINITSID_KERNEL in case the cred argument is NULL.
>
> Most of the callers are updated to pass current_cred() as the cred
> pointer, thus maintaining the same behavior. The following callers are
> modified to pass NULL as the cred pointer instead:
> 1. arch/powerpc/xmon/xmon.c
> Seems to be some interactive debugging facility. It appears that
> the lockdown hook is called from interrupt context here, so it
> should be more appropriate to request a global lockdown decision.
> 2. fs/tracefs/inode.c:tracefs_create_file()
> Here the call is used to prevent creating new tracefs entries when
> the kernel is locked down. Assumes that locking down is one-way -
> i.e. if the hook returns non-zero once, it will never return zero
> again, thus no point in creating these files. Also, the hook is
> often called by a module's init function when it is loaded by
> userspace, where it doesn't make much sense to do a check against
> the current task's creds, since the task itself doesn't actually
> use the tracing functionality (i.e. doesn't breach lockdown), just
> indirectly makes some new tracepoints available to whoever is
> authorized to use them.
> 3. net/xfrm/xfrm_user.c:copy_to_user_*()
> Here a cryptographic secret is redacted based on the value returned
> from the hook. There are two possible actions that may lead here:
> a) A netlink message XFRM_MSG_GETSA with NLM_F_DUMP set - here the
> task context is relevant, since the dumped data is sent back to
> the current task.
> b) When adding/deleting/updating an SA via XFRM_MSG_xxxSA, the
> dumped SA is broadcasted to tasks subscribed to XFRM events -
> here the current task context is not relevant as it doesn't
> represent the tasks that could potentially see the secret.
> It doesn't seem worth it to try to keep using the current task's
> context in the a) case, since the eventual data leak can be
> circumvented anyway via b), plus there is no way for the task to
> indicate that it doesn't care about the actual key value, so the
> check could generate a lot of "false alert" denials with SELinux.
> Thus, let's pass NULL instead of current_cred() here faute de
> mieux.
>
> Improvements-suggested-by: Casey Schaufler
> Improvements-suggested-by: Paul Moore
> Fixes: 59438b46471a ("security,lockdown,selinux: implement SELinux lockdown")
> Acked-by: Dan Williams [cxl]
> Acked-by: Steffen Klassert [xfrm]
> Signed-off-by: Ondrej Mosnacek
> ---
>
> v4:
> - rebase on top of TODO
> - fix rebase conflicts:
> * drivers/cxl/pci.c
> - trivial: the lockdown reason was corrected in mainline
> * kernel/bpf/helpers.c, kernel/trace/bpf_trace.c
> - trivial: LOCKDOWN_BPF_READ was renamed to LOCKDOWN_BPF_READ_KERNEL
> in mainline
> * kernel/power/hibernate.c
> - trivial: !secretmem_active() was added to the condition in
> hibernation_available()
> - cover new security_locked_down() call in kernel/bpf/helpers.c
> (LOCKDOWN_BPF_WRITE_USER in BPF_FUNC_probe_write_user case)
>
> v3: https://lore.kernel.org/lkml/20210616085118.1141101-1-omosn...@redhat.com/
> - add the cred argument to security_locked_down() and adapt all callers
> - keep using current_cred() in BPF, as the hook calls have been shifted
> to program load time (commit ff40e51043af ("bpf, lockdown, audit: Fix
> buggy SELinux lockdown permission checks"))
> - in SELinux, don't ignore hook calls where cred == NULL, but use
> SECINITSID_KERNEL as the subject instead
> - update explanations in the commit message
>
> v2: https://lore.kernel.org/lkml/20210517092006.803332-1-omosn...@redhat.com/
> - change to a single hook based on suggestions by Casey Scha
Re: [PATCH v3] lockdown,selinux: fix wrong subject in some SELinux lockdown checks
On Tue, Aug 31, 2021 at 2:58 PM Dan Williams wrote: > On Tue, Aug 31, 2021 at 6:53 AM Paul Moore wrote: > > On Tue, Aug 31, 2021 at 5:09 AM Ondrej Mosnacek wrote: > > > On Sat, Jun 19, 2021 at 12:18 AM Dan Williams > > > wrote: > > > > On Wed, Jun 16, 2021 at 1:51 AM Ondrej Mosnacek > > > > wrote: > > > > ... > > > > > > > diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c > > > > > index 2acc6173da36..c1747b6555c7 100644 > > > > > --- a/drivers/cxl/mem.c > > > > > +++ b/drivers/cxl/mem.c > > > > > @@ -568,7 +568,7 @@ static bool cxl_mem_raw_command_allowed(u16 > > > > > opcode) > > > > > if (!IS_ENABLED(CONFIG_CXL_MEM_RAW_COMMANDS)) > > > > > return false; > > > > > > > > > > - if (security_locked_down(LOCKDOWN_NONE)) > > > > > + if (security_locked_down(current_cred(), LOCKDOWN_NONE)) > > > > > > > > Acked-by: Dan Williams > > > > > > > > ...however that usage looks wrong. The expectation is that if kernel > > > > integrity protections are enabled then raw command access should be > > > > disabled. So I think that should be equivalent to LOCKDOWN_PCI_ACCESS > > > > in terms of the command capabilities to filter. > > > > > > Yes, the LOCKDOWN_NONE seems wrong here... but it's a pre-existing bug > > > and I didn't want to go down yet another rabbit hole trying to fix it. > > > I'll look at this again once this patch is settled - it may indeed be > > > as simple as replacing LOCKDOWN_NONE with LOCKDOWN_PCI_ACCESS. > > > > At this point you should be well aware of my distaste for merging > > patches that have known bugs in them. Yes, this is a pre-existing > > condition, but it seems well within the scope of this work to address > > it as well. > > > > This isn't something that is going to get merged while the merge > > window is open, so at the very least you've got almost two weeks to > > sort this out - please do that. > > Yes, apologies, I should have sent the fix shortly after noticing the > problem. I'll get the CXL bug fix out of the way so Ondrej can move > this along. Thanks Dan. -- paul moore www.paul-moore.com
Re: [PATCH v3] lockdown,selinux: fix wrong subject in some SELinux lockdown checks
On Tue, Aug 31, 2021 at 5:09 AM Ondrej Mosnacek wrote: > On Sat, Jun 19, 2021 at 12:18 AM Dan Williams > wrote: > > On Wed, Jun 16, 2021 at 1:51 AM Ondrej Mosnacek wrote: ... > > > diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c > > > index 2acc6173da36..c1747b6555c7 100644 > > > --- a/drivers/cxl/mem.c > > > +++ b/drivers/cxl/mem.c > > > @@ -568,7 +568,7 @@ static bool cxl_mem_raw_command_allowed(u16 opcode) > > > if (!IS_ENABLED(CONFIG_CXL_MEM_RAW_COMMANDS)) > > > return false; > > > > > > - if (security_locked_down(LOCKDOWN_NONE)) > > > + if (security_locked_down(current_cred(), LOCKDOWN_NONE)) > > > > Acked-by: Dan Williams > > > > ...however that usage looks wrong. The expectation is that if kernel > > integrity protections are enabled then raw command access should be > > disabled. So I think that should be equivalent to LOCKDOWN_PCI_ACCESS > > in terms of the command capabilities to filter. > > Yes, the LOCKDOWN_NONE seems wrong here... but it's a pre-existing bug > and I didn't want to go down yet another rabbit hole trying to fix it. > I'll look at this again once this patch is settled - it may indeed be > as simple as replacing LOCKDOWN_NONE with LOCKDOWN_PCI_ACCESS. At this point you should be well aware of my distaste for merging patches that have known bugs in them. Yes, this is a pre-existing condition, but it seems well within the scope of this work to address it as well. This isn't something that is going to get merged while the merge window is open, so at the very least you've got almost two weeks to sort this out - please do that. -- paul moore www.paul-moore.com
Re: [PATCH v3] lockdown,selinux: fix wrong subject in some SELinux lockdown checks
On Tue, Aug 31, 2021 at 5:08 AM Ondrej Mosnacek wrote: > Can we move this forward somehow, please? As mentioned previously, I can merge this via the SELinux tree but I need to see some ACKs from the other subsystems first, not to mention some resolution to the outstanding questions. -- paul moore www.paul-moore.com
Re: [PATCH v2 RESEND] powerpc/audit: Convert powerpc to AUDIT_ARCH_COMPAT_GENERIC
On Thu, Aug 26, 2021 at 10:37 AM Michael Ellerman wrote:
> Paul Moore writes:
> > On Tue, Aug 24, 2021 at 1:11 PM Christophe Leroy
> > wrote:
> >> Le 24/08/2021 à 16:47, Paul Moore a écrit :
> >> > On Tue, Aug 24, 2021 at 9:36 AM Christophe Leroy
> >> > wrote:
> >> >>
> >> >> Commit e65e1fc2d24b ("[PATCH] syscall class hookup for all normal
> >> >> targets") added generic support for AUDIT but that didn't include
> >> >> support for bi-arch like powerpc.
> >> >>
> >> >> Commit 4b58841149dc ("audit: Add generic compat syscall support")
> >> >> added generic support for bi-arch.
> >> >>
> >> >> Convert powerpc to that bi-arch generic audit support.
> >> >>
> >> >> Cc: Paul Moore
> >> >> Cc: Eric Paris
> >> >> Signed-off-by: Christophe Leroy
> >> >> ---
> >> >> Resending v2 with Audit people in Cc
> >> >>
> >> >> v2:
> >> >> - Missing 'git add' for arch/powerpc/include/asm/unistd32.h
> >> >> - Finalised commit description
> >> >> ---
> >> >> arch/powerpc/Kconfig| 5 +-
> >> >> arch/powerpc/include/asm/unistd32.h | 7 +++
> >> >> arch/powerpc/kernel/Makefile| 3 --
> >> >> arch/powerpc/kernel/audit.c | 84 -
> >> >> arch/powerpc/kernel/compat_audit.c | 44 ---
> >> >> 5 files changed, 8 insertions(+), 135 deletions(-)
> >> >> create mode 100644 arch/powerpc/include/asm/unistd32.h
> >> >> delete mode 100644 arch/powerpc/kernel/audit.c
> >> >> delete mode 100644 arch/powerpc/kernel/compat_audit.c
> >> >
> >> > Can you explain, in detail please, the testing you have done to verify
> >> > this patch?
> >> >
> >>
> >> I built ppc64_defconfig and checked that the generated code is
> >> functionnaly equivalent.
> >>
> >> ppc32_classify_syscall() is exactly the same as
> >> audit_classify_compat_syscall() except that the
> >> later takes the syscall as second argument (ie in r4) whereas the former
> >> takes it as first argument
> >> (ie in r3).
> >>
> >> audit_classify_arch() and powerpc audit_classify_syscall() are slightly
> >> different between the
> >> powerpc version and the generic version because the powerpc version checks
> >> whether it is
> >> AUDIT_ARCH_PPC or not (ie value 20), while the generic one checks whether
> >> it has bit
> >> __AUDIT_ARCH_64BIT set or not (__AUDIT_ARCH_64BIT is the sign bit of a
> >> word), but taking into
> >> account that the abi is either AUDIT_ARCH_PPC, AUDIT_ARCH_PPC64 or
> >> AUDIT_ARCH_PPC64LE, the result is
> >> the same.
> >>
> >> If you are asking I guess you saw something wrong ?
> >
> > I was asking because I didn't see any mention of testing, and when you
> > are enabling something significant like this it is nice to see that it
> > has been verified to work :)
> >
> > While binary dumps and comparisons are nice, it is always good to see
> > verification from a test suite. I don't have access to the necessary
> > hardware to test this, but could you verify that the audit-testsuite
> > passes on your test system with your patches applied?
> >
> > * https://github.com/linux-audit/audit-testsuite
>
> I tested on ppc64le. Both before and after the patch I get the result
> below.
>
> So I guess the patch is OK, but maybe we have some existing issue.
>
> I had a bit of a look at the test code, but my perl is limited. I think
> it was running the command below, and it returned "", but
> not really sure what that means.
If it makes you feel any better, my perl is *very* limited; thankfully
this isn't my first time looking at that test :)
It's a little odd, but after some basic sanity tests at the top, the
test sets a watch on a file, /tmp/, and tells the kernel
to generate audit records for any syscall that operates on that file.
It then creates, and removes, a series of exclude audit filters to
check if the exclude filtering is working as expected, e.g. when
syscall filtering is excluded there should be no syscall records in
the audit log.
In the case you describe, it looks like it looks like the audit
exclude filter is removed (that's what line 147 does), the
/tmp/ file is removed (line 152), and then we check to
se
Re: [PATCH v2 RESEND] powerpc/audit: Convert powerpc to AUDIT_ARCH_COMPAT_GENERIC
On Tue, Aug 24, 2021 at 1:11 PM Christophe Leroy
wrote:
> Le 24/08/2021 à 16:47, Paul Moore a écrit :
> > On Tue, Aug 24, 2021 at 9:36 AM Christophe Leroy
> > wrote:
> >>
> >> Commit e65e1fc2d24b ("[PATCH] syscall class hookup for all normal
> >> targets") added generic support for AUDIT but that didn't include
> >> support for bi-arch like powerpc.
> >>
> >> Commit 4b58841149dc ("audit: Add generic compat syscall support")
> >> added generic support for bi-arch.
> >>
> >> Convert powerpc to that bi-arch generic audit support.
> >>
> >> Cc: Paul Moore
> >> Cc: Eric Paris
> >> Signed-off-by: Christophe Leroy
> >> ---
> >> Resending v2 with Audit people in Cc
> >>
> >> v2:
> >> - Missing 'git add' for arch/powerpc/include/asm/unistd32.h
> >> - Finalised commit description
> >> ---
> >> arch/powerpc/Kconfig| 5 +-
> >> arch/powerpc/include/asm/unistd32.h | 7 +++
> >> arch/powerpc/kernel/Makefile| 3 --
> >> arch/powerpc/kernel/audit.c | 84 -
> >> arch/powerpc/kernel/compat_audit.c | 44 ---
> >> 5 files changed, 8 insertions(+), 135 deletions(-)
> >> create mode 100644 arch/powerpc/include/asm/unistd32.h
> >> delete mode 100644 arch/powerpc/kernel/audit.c
> >> delete mode 100644 arch/powerpc/kernel/compat_audit.c
> >
> > Can you explain, in detail please, the testing you have done to verify
> > this patch?
> >
>
> I built ppc64_defconfig and checked that the generated code is functionnaly
> equivalent.
>
> ppc32_classify_syscall() is exactly the same as
> audit_classify_compat_syscall() except that the
> later takes the syscall as second argument (ie in r4) whereas the former
> takes it as first argument
> (ie in r3).
>
> audit_classify_arch() and powerpc audit_classify_syscall() are slightly
> different between the
> powerpc version and the generic version because the powerpc version checks
> whether it is
> AUDIT_ARCH_PPC or not (ie value 20), while the generic one checks whether it
> has bit
> __AUDIT_ARCH_64BIT set or not (__AUDIT_ARCH_64BIT is the sign bit of a word),
> but taking into
> account that the abi is either AUDIT_ARCH_PPC, AUDIT_ARCH_PPC64 or
> AUDIT_ARCH_PPC64LE, the result is
> the same.
>
> If you are asking I guess you saw something wrong ?
I was asking because I didn't see any mention of testing, and when you
are enabling something significant like this it is nice to see that it
has been verified to work :)
While binary dumps and comparisons are nice, it is always good to see
verification from a test suite. I don't have access to the necessary
hardware to test this, but could you verify that the audit-testsuite
passes on your test system with your patches applied?
* https://github.com/linux-audit/audit-testsuite
--
paul moore
www.paul-moore.com
Re: [PATCH v2 RESEND] powerpc/audit: Convert powerpc to AUDIT_ARCH_COMPAT_GENERIC
On Tue, Aug 24, 2021 at 9:36 AM Christophe Leroy
wrote:
>
> Commit e65e1fc2d24b ("[PATCH] syscall class hookup for all normal
> targets") added generic support for AUDIT but that didn't include
> support for bi-arch like powerpc.
>
> Commit 4b58841149dc ("audit: Add generic compat syscall support")
> added generic support for bi-arch.
>
> Convert powerpc to that bi-arch generic audit support.
>
> Cc: Paul Moore
> Cc: Eric Paris
> Signed-off-by: Christophe Leroy
> ---
> Resending v2 with Audit people in Cc
>
> v2:
> - Missing 'git add' for arch/powerpc/include/asm/unistd32.h
> - Finalised commit description
> ---
> arch/powerpc/Kconfig| 5 +-
> arch/powerpc/include/asm/unistd32.h | 7 +++
> arch/powerpc/kernel/Makefile| 3 --
> arch/powerpc/kernel/audit.c | 84 -
> arch/powerpc/kernel/compat_audit.c | 44 ---
> 5 files changed, 8 insertions(+), 135 deletions(-)
> create mode 100644 arch/powerpc/include/asm/unistd32.h
> delete mode 100644 arch/powerpc/kernel/audit.c
> delete mode 100644 arch/powerpc/kernel/compat_audit.c
Can you explain, in detail please, the testing you have done to verify
this patch?
--
paul moore
www.paul-moore.com
Re: [PATCH v4 1/3] audit: replace magic audit syscall class numbers with macros
On Wed, May 19, 2021 at 4:01 PM Richard Guy Briggs wrote:
>
> Replace audit syscall class magic numbers with macros.
>
> This required putting the macros into new header file
> include/linux/auditsc_classmacros.h since the syscall macros were
> included for both 64 bit and 32 bit in any compat code, causing
> redefinition warnings.
>
> Signed-off-by: Richard Guy Briggs
> Link:
> https://lore.kernel.org/r/2300b1083a32aade7ae7efb95826e8f3f260b1df.1621363275.git@redhat.com
> ---
> MAINTAINERS | 1 +
> arch/alpha/kernel/audit.c | 8
> arch/ia64/kernel/audit.c| 8
> arch/parisc/kernel/audit.c | 8
> arch/parisc/kernel/compat_audit.c | 9 +
> arch/powerpc/kernel/audit.c | 10 +-
> arch/powerpc/kernel/compat_audit.c | 11 ++-
> arch/s390/kernel/audit.c| 10 +-
> arch/s390/kernel/compat_audit.c | 11 ++-
> arch/sparc/kernel/audit.c | 10 +-
> arch/sparc/kernel/compat_audit.c| 11 ++-
> arch/x86/ia32/audit.c | 11 ++-
> arch/x86/kernel/audit_64.c | 8
> include/linux/audit.h | 1 +
> include/linux/auditsc_classmacros.h | 23 +++
> kernel/auditsc.c| 12 ++--
> lib/audit.c | 10 +-
> lib/compat_audit.c | 11 ++-
> 18 files changed, 102 insertions(+), 71 deletions(-)
> create mode 100644 include/linux/auditsc_classmacros.h
...
> diff --git a/include/linux/auditsc_classmacros.h
> b/include/linux/auditsc_classmacros.h
> new file mode 100644
> index ..18757d270961
> --- /dev/null
> +++ b/include/linux/auditsc_classmacros.h
> @@ -0,0 +1,23 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/* auditsc_classmacros.h -- Auditing support syscall macros
> + *
> + * Copyright 2021 Red Hat Inc., Durham, North Carolina.
> + * All Rights Reserved.
> + *
> + * Author: Richard Guy Briggs
> + */
> +#ifndef _LINUX_AUDITSCM_H_
> +#define _LINUX_AUDITSCM_H_
> +
> +enum auditsc_class_t {
> + AUDITSC_NATIVE = 0,
> + AUDITSC_COMPAT,
> + AUDITSC_OPEN,
> + AUDITSC_OPENAT,
> + AUDITSC_SOCKETCALL,
> + AUDITSC_EXECVE,
> +
> + AUDITSC_NVALS /* count */
> +};
> +
> +#endif
My apologies Richard, for some reason I had it in my mind that this
series was waiting on you to answer a question and/or respin; however,
now that I'm clearing my patch queues looking for any stragglers I see
that isn't the case. Looking over the patchset I think it looks okay
to me, my only concern is that "auditsc_classmacros.h" is an awfully
specific header file name and could prove to be annoying if we want to
add to it in the future. What do you think about something like
"audit_arch.h" instead?
If that change is okay with you I can go ahead and do the rename while
I'm merging the patches, I'll consider it penance for letting this
patchset sit for so long :/
--
paul moore
www.paul-moore.com
Re: [PATCH v3] lockdown,selinux: fix wrong subject in some SELinux lockdown checks
On Sat, Jun 19, 2021 at 1:00 PM Thomas Gleixner wrote:
> On Wed, Jun 16 2021 at 10:51, Ondrej Mosnacek wrote:
> > diff --git a/arch/x86/mm/testmmiotrace.c b/arch/x86/mm/testmmiotrace.c
> > index bda73cb7a044..c43a13241ae8 100644
> > --- a/arch/x86/mm/testmmiotrace.c
> > +++ b/arch/x86/mm/testmmiotrace.c
> > @@ -116,7 +116,7 @@ static void do_test_bulk_ioremapping(void)
> > static int __init init(void)
> > {
> > unsigned long size = (read_far) ? (8 << 20) : (16 << 10);
> > - int ret = security_locked_down(LOCKDOWN_MMIOTRACE);
> > + int ret = security_locked_down(current_cred(), LOCKDOWN_MMIOTRACE);
>
> I have no real objection to those patches, but it strikes me odd that
> out of the 62 changed places 58 have 'current_cred()' and 4 have NULL as
> argument.
>
> I can't see why this would ever end up with anything else than
> current_cred() or NULL and NULL being the 'special' case. So why not
> having security_locked_down_no_cred() and make current_cred() implicit
> for security_locked_down() which avoids most of the churn and just makes
> the special cases special. I might be missing something though.
Unfortunately it is not uncommon for kernel subsystems to add, move,
or otherwise play around with LSM hooks without checking with the LSM
folks; generally this is okay, but there have been a few problems in
the past and I try to keep that in mind when we are introducing new
hooks or modifying existing ones. If we have two LSM hooks for
roughly the same control point it has the potential to cause
confusion, e.g. do I use the "normal" or the "no_cred" version? What
if I don't want to pass a credential, can I just use "no_cred"? My
thinking with the single, always-pass-a-cred function is that callers
don't have to worry about choosing from multiple, similar hooks and
they know they need to pass a cred which hopefully gets them thinking
about what cred is appropriate. It's not foolproof, but I believe the
single hook approach will be less prone to accidents ... or so I hope
:)
--
paul moore
www.paul-moore.com
Re: [PATCH v3] lockdown,selinux: fix wrong subject in some SELinux lockdown checks
On Wed, Jun 16, 2021 at 4:51 AM Ondrej Mosnacek wrote:
>
> Commit 59438b46471a ("security,lockdown,selinux: implement SELinux
> lockdown") added an implementation of the locked_down LSM hook to
> SELinux, with the aim to restrict which domains are allowed to perform
> operations that would breach lockdown.
>
> However, in several places the security_locked_down() hook is called in
> situations where the current task isn't doing any action that would
> directly breach lockdown, leading to SELinux checks that are basically
> bogus.
>
> To fix this, add an explicit struct cred pointer argument to
> security_lockdown() and define NULL as a special value to pass instead
> of current_cred() in such situations. LSMs that take the subject
> credentials into account can then fall back to some default or ignore
> such calls altogether. In the SELinux lockdown hook implementation, use
> SECINITSID_KERNEL in case the cred argument is NULL.
>
> Most of the callers are updated to pass current_cred() as the cred
> pointer, thus maintaining the same behavior. The following callers are
> modified to pass NULL as the cred pointer instead:
> 1. arch/powerpc/xmon/xmon.c
> Seems to be some interactive debugging facility. It appears that
> the lockdown hook is called from interrupt context here, so it
> should be more appropriate to request a global lockdown decision.
> 2. fs/tracefs/inode.c:tracefs_create_file()
> Here the call is used to prevent creating new tracefs entries when
> the kernel is locked down. Assumes that locking down is one-way -
> i.e. if the hook returns non-zero once, it will never return zero
> again, thus no point in creating these files. Also, the hook is
> often called by a module's init function when it is loaded by
> userspace, where it doesn't make much sense to do a check against
> the current task's creds, since the task itself doesn't actually
> use the tracing functionality (i.e. doesn't breach lockdown), just
> indirectly makes some new tracepoints available to whoever is
> authorized to use them.
> 3. net/xfrm/xfrm_user.c:copy_to_user_*()
> Here a cryptographic secret is redacted based on the value returned
> from the hook. There are two possible actions that may lead here:
> a) A netlink message XFRM_MSG_GETSA with NLM_F_DUMP set - here the
> task context is relevant, since the dumped data is sent back to
> the current task.
> b) When adding/deleting/updating an SA via XFRM_MSG_xxxSA, the
> dumped SA is broadcasted to tasks subscribed to XFRM events -
> here the current task context is not relevant as it doesn't
> represent the tasks that could potentially see the secret.
> It doesn't seem worth it to try to keep using the current task's
> context in the a) case, since the eventual data leak can be
> circumvented anyway via b), plus there is no way for the task to
> indicate that it doesn't care about the actual key value, so the
> check could generate a lot of "false alert" denials with SELinux.
> Thus, let's pass NULL instead of current_cred() here faute de
> mieux.
>
> Improvements-suggested-by: Casey Schaufler
> Improvements-suggested-by: Paul Moore
> Fixes: 59438b46471a ("security,lockdown,selinux: implement SELinux lockdown")
> Signed-off-by: Ondrej Mosnacek
This seems reasonable to me, but before I merge it into the SELinux
tree I think it would be good to get some ACKs from the relevant
subsystem folks. I don't believe we ever saw a response to the last
question for the PPC folks, did we?
> ---
>
> v3:
> - add the cred argument to security_locked_down() and adapt all callers
> - keep using current_cred() in BPF, as the hook calls have been shifted
> to program load time (commit ff40e51043af ("bpf, lockdown, audit: Fix
> buggy SELinux lockdown permission checks"))
> - in SELinux, don't ignore hook calls where cred == NULL, but use
> SECINITSID_KERNEL as the subject instead
> - update explanations in the commit message
>
> v2: https://lore.kernel.org/lkml/20210517092006.803332-1-omosn...@redhat.com/
> - change to a single hook based on suggestions by Casey Schaufler
>
> v1: https://lore.kernel.org/lkml/20210507114048.138933-1-omosn...@redhat.com/
>
> arch/powerpc/xmon/xmon.c | 4 ++--
> arch/x86/kernel/ioport.c | 4 ++--
> arch/x86/kernel/msr.c| 4 ++--
> arch/x86/mm/testmmiotrace.c | 2 +-
> drivers/acpi/acpi_configfs.c | 2 +-
> drivers/acpi/custom_method.c | 2 +-
> drivers/acpi/osl.c | 3 ++-
> drivers/acpi/tables.c| 2 +-
&g
Re: [PATCH v2] lockdown,selinux: avoid bogus SELinux lockdown permission checks
On Tue, Jun 8, 2021 at 7:02 AM Ondrej Mosnacek wrote: > On Thu, Jun 3, 2021 at 7:46 PM Paul Moore wrote: ... > > It sounds an awful lot like the lockdown hook is in the wrong spot. > > It sounds like it would be a lot better to relocate the hook than > > remove it. > > I don't see how you would solve this by moving the hook. Where do you > want to relocate it? Wherever it makes sense. Based on your comments it really sounded like the hook was in a bad spot and since your approach in a lot of this had been to remove or disable hooks I wanted to make sure that relocating the hook was something you had considered. Thankfully it sounds like you have considered moving the hook - that's good. > The main obstacle is that the message containing > the SA dump is sent to consumers via a simple netlink broadcast, which > doesn't provide a facility to redact the SA secret on a per-consumer > basis. I can't see any way to make the checks meaningful for SELinux > without a major overhaul of the broadcast logic. Fair enough. -- paul moore www.paul-moore.com
Re: [PATCH v2] lockdown,selinux: avoid bogus SELinux lockdown permission checks
On Sat, Jun 5, 2021 at 2:17 PM Linus Torvalds wrote: > On Sat, Jun 5, 2021 at 11:11 AM Casey Schaufler > wrote: > > > > You have fallen into a common fallacy. The fact that the "code runs" > > does not assure that the "system works right". In the security world > > we face this all the time, often with performance expectations. In this > > case the BPF design has failed [..] > > I think it's the lockdown patches that have failed. They did the wrong > thing, they didn't work, > > The report in question is for a regression. > > THERE ARE NO VALID ARGUMENTS FOR REGRESSIONS. To think I was worried we might end this thread without a bit of CAPS LOCK, whew! :) I don't think anyone in this discussion, even Casey's last comment, was denying that there was a problem. The discussion and the disagreements were about what a "proper" fix would be, and how one might implement that fix; of course there were different ideas of "proper" and implementations vary even when people agree, so things were a bit of a mess. If you want to get upset and shouty, I think there are a few things spread across the subsystems involved that would be worthy targets, but to say that Casey, myself, or anyone else who plays under security/ denied the problem in this thread is not fair, or correct, in my opinion. > Honestly, security people need to understand that "not working" is not > a success case of security. It's a failure case. I can't pretend to know what all of the "security people" are thinking, but I can say with a good degree of certainty that my goal is not to crash, panic, kill, or otherwise disable a user's system. When it comes to things like the LSM hooks, my goal is to try and make sure we have the right hooks in the right places so that admins and users have the tools they need to control access to their data and systems in the way that they choose. Sometimes this puts us at odds with other subsystems in the kernel, we saw that in this thread, but that's to be expected anytime you have competing priorities. The important part is that eventually we figure out some way to move forward, and the fact that we are still all making progress and putting out new kernel releases is proof that we are finding a way. That's what matters to me, and if I was forced to guess, I would imagine that matters quite a lot to most of us here. -- paul moore www.paul-moore.com
Re: [PATCH v2] lockdown,selinux: avoid bogus SELinux lockdown permission checks
On Fri, Jun 4, 2021 at 8:08 PM Alexei Starovoitov wrote: > On Fri, Jun 4, 2021 at 4:34 PM Paul Moore wrote: > > > > > Again, the problem is not limited to BPF at all. kprobes is doing > > > register- > > > time hooks which are equivalent to the one of BPF. Anything in run-time > > > trying to prevent probe_read_kernel by kprobes or BPF is broken by design. > > > > Not being an expert on kprobes I can't really comment on that, but > > right now I'm focused on trying to make things work for the BPF > > helpers. I suspect that if we can get the SELinux lockdown > > implementation working properly for BPF the solution for kprobes won't > > be far off. > > Paul, Hi Alexei, > Both kprobe and bpf can call probe_read_kernel==copy_from_kernel_nofault > from all contexts. > Including NMI. Thanks, that is helpful. In hindsight it should have been obvious that kprobe/BPF would offer to insert code into the NMI handlers, but I don't recall it earlier in the discussion, it's possible I simply missed the mention. > Most of audit_log_* is not acceptable. > Just removing a wakeup is not solving anything. That's not really fair now is it? Removing the wakeups in audit_log_start() and audit_log_end() does solve some problems, although not all of them (i.e. the NMI problem being the 800lb gorilla). Because of the NMI case we're not going to solve the LSM/audit case anytime soon so it looks like we are going to have to fall back to the patch Daniel proposed. Acked-by: Paul Moore -- paul moore www.paul-moore.com
Re: [PATCH v2] lockdown,selinux: avoid bogus SELinux lockdown permission checks
On Fri, Jun 4, 2021 at 2:02 PM Daniel Borkmann wrote:
>
> On 6/4/21 6:50 AM, Paul Moore wrote:
> > On Thu, Jun 3, 2021 at 2:53 PM Daniel Borkmann wrote:
> [...]
> >> I did run this entire discussion by both of the other BPF co-maintainers
> >> (Alexei, Andrii, CC'ed) and together we did further brainstorming on the
> >> matter on how we could solve this, but couldn't find a sensible & clean
> >> solution so far.
> >
> > Before I jump into the patch below I just want to say that I
> > appreciate you looking into solutions on the BPF side of things.
> > However, I voted "no" on this patch previously and since you haven't
> > really changed it, my "no"/NACK vote remains, at least until we
> > exhaust a few more options.
>
> Just to set the record straight, you previously did neither ACK nor NACK it.
I think I said it wasn't a great solution. Perhaps I should have been
more clear, but I don't like NACK'ing things while we are still
hashing out possible solutions on the lists.
>From my perspective NACK'ing is pretty harsh and I try to leave that
as a last resort.
> And
> again, as summarized earlier, this patch is _fixing_ the majority of the
> damage
> caused by 59438b46471a for at least the BPF side of things where users run
> into this,
> Ondrej the rest. Everything else can be discussed on top, and so far it seems
> there
> is no clean solution in front of us either, not even speaking of one that is
> small
> and suitable for _stable_ trees. Let me reiterate where we are: it's not that
> the
> original implementation in 9d1f8be5cf42 ("bpf: Restrict bpf when kernel
> lockdown is
> in confidentiality mode") was broken, it's that the later added _SELinux_
> locked_down
> hook implementation that is broken, and not other LSMs.
I think there are still options for how to solve this moving forward,
more on that at the end of the email, and I would like to see if we
can chase down some of those ideas first. Maybe the ideas aren't
practical, or maybe they are practical but not from a -stable
perspective; we/I don't know until we talk about it. Based on
previous experience I'm afraid to ACK a quick-fix without some
discussion about the proper long-term fix. If the long-term fix isn't
suitable for -stable, then we can take a smaller fix just for the
stable trees.
> Now you're trying to retrofittingly
> ask us for hacks at all costs just because of /a/ broken LSM implementation.
This is starting to get a little off the rails now isn't it? Daniel
you've always come across as a reasonable person to me, but phrasing
things like that is inflammatory at best.
Could the SELinux lockdown hooks have been done better, of course, I
don't think we are debating that anymore. New functionality, checks,
etc. are added to the kernel all the time, and because we're all human
we screw that up on occasion. The important part is that we come
together to fix it, which is what I think we're trying to do here
(it's what I'm trying to do here).
> Maybe
> another avenue is to just swallow the pill and revert 59438b46471a since it
> made
> assumptions that don't work /generally/. And the use case for an application
> runtime
> policy change in particular in case of lockdown where users only have 3
> choices is
> extremely tiny/rare, if it's not then something is very wrong in your
> deployment.
> Let me ask you this ... are you also planning to address *all* the other
> cases inside
> the kernel? If your answer is no, then this entire discussion is pointless.
Um, yes? We were talking about that earlier in the thread before the
BPF portions of the fix started to dominate the discussion. Just
because the BPF portion of the fix has dominated the discussion
recently doesn't mean the other issues aren't going to be addressed.
When stuff is busted, I work to fix it. I think everyone here does.
> >> [PATCH] bpf, lockdown, audit: Fix buggy SELinux lockdown permission checks
> >>
> >> Commit 59438b46471a ("security,lockdown,selinux: implement SELinux
> >> lockdown")
> >> added an implementation of the locked_down LSM hook to SELinux, with the
> >> aim
> >> to restrict which domains are allowed to perform operations that would
> >> breach
> >> lockdown. This is indirectly also getting audit subsystem involved to
> >> report
> >> events. The latter is problematic, as reported by Ondrej and Serhei, since
> >> it
> >> can bring down the whole system via audit:
> >>
> >> 1) The audit events that are triggered due to calls to
> >> security_locked_down()
> >>can OOM kill a machine, see below details [0].
> >>
> &
Re: [PATCH v2] lockdown,selinux: avoid bogus SELinux lockdown permission checks
On Thu, Jun 3, 2021 at 2:53 PM Daniel Borkmann wrote:
> On 6/2/21 5:13 PM, Paul Moore wrote:
> [...]
> > Help me out here, is your answer that the access check can only be
> > done at BPF program load time? That isn't really a solution from a
> > SELinux perspective as far as I'm concerned.
>
> That is the current answer. The unfortunate irony is that 59438b46471a
> ("security,lockdown,selinux: implement SELinux lockdown") broke this in
> the first place. W/o the SELinux hook implementation it would have been
> working just fine at runtime, but given it's UAPI since quite a while
> now, that ship has sailed.
Explaining the other side of the "unfortunate irony ..." comment is
going to take us in a direction that isn't very constructive so I'm
going to skip past that now and simply say that if there was better
cooperation across subsystems, especially with the LSM folks, a lot of
this pain could be mitigated.
... and yes I said "mitigated", I'm not foolish to think the pain
could be avoided entirely ;)
> > I understand the ideas I've tossed out aren't practical from a BPF
> > perspective, but it would be nice if we could find something that does
> > work. Surely you BPF folks can think of some way to provide a
> > runtime, not load time, check?
>
> I did run this entire discussion by both of the other BPF co-maintainers
> (Alexei, Andrii, CC'ed) and together we did further brainstorming on the
> matter on how we could solve this, but couldn't find a sensible & clean
> solution so far.
Before I jump into the patch below I just want to say that I
appreciate you looking into solutions on the BPF side of things.
However, I voted "no" on this patch previously and since you haven't
really changed it, my "no"/NACK vote remains, at least until we
exhaust a few more options.
> [PATCH] bpf, lockdown, audit: Fix buggy SELinux lockdown permission checks
>
> Commit 59438b46471a ("security,lockdown,selinux: implement SELinux lockdown")
> added an implementation of the locked_down LSM hook to SELinux, with the aim
> to restrict which domains are allowed to perform operations that would breach
> lockdown. This is indirectly also getting audit subsystem involved to report
> events. The latter is problematic, as reported by Ondrej and Serhei, since it
> can bring down the whole system via audit:
>
>1) The audit events that are triggered due to calls to
> security_locked_down()
> can OOM kill a machine, see below details [0].
>
>2) It also seems to be causing a deadlock via
> avc_has_perm()/slow_avc_audit()
> when trying to wake up kauditd, for example, when using
> trace_sched_switch()
> tracepoint, see details in [1]. Triggering this was not via some
> hypothetical
> corner case, but with existing tools like runqlat & runqslower from
> bcc, for
> example, which make use of this tracepoint. Rough call sequence goes
> like:
>
> rq_lock(rq) -> -+
> trace_sched_switch() -> |
> bpf_prog_xyz() -> +-> deadlock
> selinux_lockdown() -> |
> audit_log_end() -> |
> wake_up_interruptible() ->|
> try_to_wake_up() -> |
> rq_lock(rq) --+
Since BPF is a bit of chaotic nightmare in the sense that it basically
out-of-tree kernel code that can be called from anywhere and do pretty
much anything; it presents quite the challenge for those of us worried
about LSM access controls.
You and the other BPF folks have investigated ways in which BPF might
be able to disable helper functions allowing us to do proper runtime
access checks but haven't been able to make it work, which brings this
patch up yet again. I'm not a fan of this patch as it basically
allows BPF programs to side-step any changes to the security policy
once the BPF programs have been loaded; this is Not Good.
So let's look at this from a different angle. Let's look at the two
problems you mention above.
If we start with the runqueue deadlock we see the main problem is that
audit_log_end() pokes the kauditd_wait waitqueue to ensure the
kauditd_thread thread wakes up and processes the audit queue. The
audit_log_start() function does something similar, but it is
conditional on a number of factors and isn't as likely to be hit. If
we relocate these kauditd wakeup calls we can remove the deadlock in
trace_sched_switch(). In the case of CONFIG_AUDITSYSCALL=y we can
probably just move the wakeup to __audit_syscall_exit() and in the
case of CONFIG_AUDITSYSCALL=n we can likely just change the
wait_event_freezable() call in kauditd_thread to a
wait_event_freezable_
Re: [PATCH v2] lockdown,selinux: avoid bogus SELinux lockdown permission checks
On Wed, Jun 2, 2021 at 9:40 AM Ondrej Mosnacek wrote:
> On Fri, May 28, 2021 at 3:37 AM Paul Moore wrote:
> > On Mon, May 17, 2021 at 5:22 AM Ondrej Mosnacek wrote:
> > >
> > > Commit 59438b46471a ("security,lockdown,selinux: implement SELinux
> > > lockdown") added an implementation of the locked_down LSM hook to
> > > SELinux, with the aim to restrict which domains are allowed to perform
> > > operations that would breach lockdown.
> > >
> > > However, in several places the security_locked_down() hook is called in
> > > situations where the current task isn't doing any action that would
> > > directly breach lockdown, leading to SELinux checks that are basically
> > > bogus.
> > >
> > > Since in most of these situations converting the callers such that
> > > security_locked_down() is called in a context where the current task
> > > would be meaningful for SELinux is impossible or very non-trivial (and
> > > could lead to TOCTOU issues for the classic Lockdown LSM
> > > implementation), fix this by modifying the hook to accept a struct cred
> > > pointer as argument, where NULL will be interpreted as a request for a
> > > "global", task-independent lockdown decision only. Then modify SELinux
> > > to ignore calls with cred == NULL.
> >
> > I'm not overly excited about skipping the access check when cred is
> > NULL. Based on the description and the little bit that I've dug into
> > thus far it looks like using SECINITSID_KERNEL as the subject would be
> > much more appropriate. *Something* (the kernel in most of the
> > relevant cases it looks like) is requesting that a potentially
> > sensitive disclosure be made, and ignoring it seems like the wrong
> > thing to do. Leaving the access control intact also provides a nice
> > avenue to audit these requests should users want to do that.
> >
> > Those users that generally don't care can grant kernel_t all the
> > necessary permissions without much policy.
>
> Seems kind of pointless to me, but it's a relatively simple change to
> do a check against SECINITSID_KERNEL, so I don't mind doing it like
> that.
It's not pointless, the granularity isn't as great as one would like,
but it doesn't mean it is pointless. *Someone* is acting, in this
case it just happens to be the kernel. It is likely the most admins
and policy developers will not care, but some might, and we should
enable that.
> > > Since most callers will just want to pass current_cred() as the cred
> > > parameter, rename the hook to security_cred_locked_down() and provide
> > > the original security_locked_down() function as a simple wrapper around
> > > the new hook.
> >
> > I know you and Casey went back and forth on this in v1, but I agree
> > with Casey that having two LSM hooks here is a mistake. I know it
> > makes backports hard, but spoiler alert: maintaining complex software
> > over any non-trivial period of time is hard, rally hard sometimes
> > ;)
>
> Do you mean having two slots in lsm_hook_defs.h or also having two
> security_*() functions? (It's not clear to me if you're just
> reiterating disagreement with v1 or if you dislike the simplified v2
> as well.)
To be clear I don't think there should be two functions for this, just
make whatever changes are necessary to the existing
security_locked_down() LSM hook. Yes, the backport is hard. Yes, it
will touch a lot of code. Yes, those are lame excuses to not do the
right thing.
> > > The callers migrated to the new hook, passing NULL as cred:
> > > 1. arch/powerpc/xmon/xmon.c
> > > Here the hook seems to be called from non-task context and is only
> > > used for redacting some sensitive values from output sent to
> > > userspace.
> >
> > This definitely sounds like kernel_t based on the description above.
>
> Here I'm a little concerned that the hook might be called from some
> unusual interrupt, which is not masked by spin_lock_irqsave()... We
> ran into this with PMI (Platform Management Interrupt) before, see
> commit 5ae5fbd21079 ("powerpc/perf: Fix handling of privilege level
> checks in perf interrupt context"). While I can't see anything that
> would suggest something like this happening here, the whole thing is
> so foreign to me that I'm wary of making assumptions :)
>
> @Michael/PPC devs, can you confirm to us that xmon_is_locked_down() is
> only called from normal syscall/interrupt context (as opposed to
> something tricky like PMI)?
You did submit the code change so I assumed you weren't concerned
about it :)
Re: [PATCH v2] lockdown,selinux: avoid bogus SELinux lockdown permission checks
On Wed, Jun 2, 2021 at 8:40 AM Daniel Borkmann wrote: > On 6/1/21 10:47 PM, Paul Moore wrote: > > The thing I'm worried about would be the case where a LSM policy > > change requires that an existing BPF program be removed or disabled. > > I'm guessing based on the refcounting that there is not presently a > > clean way to remove a BPF program from the system, but is this > > something we could resolve? If we can't safely remove a BPF program > > from the system, can we replace/swap it with an empty/NULL BPF > > program? > > Removing progs would somehow mean destroying those references from an > async event and then /safely/ guaranteeing that nothing is accessing > them anymore. But then if policy changes once more where they would > be allowed again we would need to revert back to the original state, > which brings us to your replace/swap question with an empty/null prog. > It's not feasible either, because there are different BPF program types > and they can have different return code semantics that lead to subsequent > actions. If we were to replace them with an empty/NULL program, then > essentially this will get us into an undefined system state given it's > unclear what should be a default policy for each program type, etc. > Just to pick one simple example, outside of tracing, that comes to mind: > say, you attached a program with tc to a given device ingress hook. That > program implements firewalling functionality, and potentially deep down > in that program there is functionality to record/sample packets along > with some meta data. Part of what is exported to the ring buffer to the > user space reader may be a struct net_device field that is otherwise not > available (or at least not yet), hence it's probe-read with mentioned > helpers. If you were now to change the SELinux policy for that tc loader > application, and therefore replace/swap the progs in the kernel that were > loaded with it (given tc's lockdown policy was recorded in their sec blob) > with an empty/NULL program, then either you say allow-all or drop-all, > but either way, you break the firewalling functionality completely by > locking yourself out of the machine or letting everything through. There > is no sane way where we could reason about the context/internals of a > given program where it would be safe to replace with a simple empty/NULL > prog. Help me out here, is your answer that the access check can only be done at BPF program load time? That isn't really a solution from a SELinux perspective as far as I'm concerned. I understand the ideas I've tossed out aren't practical from a BPF perspective, but it would be nice if we could find something that does work. Surely you BPF folks can think of some way to provide a runtime, not load time, check? -- paul moore www.paul-moore.com
Re: [PATCH v2] lockdown,selinux: avoid bogus SELinux lockdown permission checks
On Mon, May 31, 2021 at 4:24 AM Daniel Borkmann wrote:
> On 5/29/21 8:48 PM, Paul Moore wrote:
> [...]
> > Daniel's patch side steps that worry by just doing the lockdown
> > permission check when the BPF program is loaded, but that isn't a
> > great solution if the policy changes afterward. I was hoping there
> > might be some way to perform the permission check as needed, but the
> > more I look the more that appears to be difficult, if not impossible
> > (once again, corrections are welcome).
>
> Your observation is correct, will try to clarify below a bit.
>
> > I'm now wondering if the right solution here is to make use of the LSM
> > notifier mechanism. I'm not yet entirely sure if this would work from
> > a BPF perspective, but I could envision the BPF subsystem registering
> > a LSM notification callback via register_blocking_lsm_notifier(), see
> > if Infiniband code as an example, and then when the LSM(s) policy
> > changes the BPF subsystem would get a notification and it could
> > revalidate the existing BPF programs and take block/remove/whatever
> > the offending BPF programs. This obviously requires a few things
> > which I'm not sure are easily done, or even possible:
> >
> > 1. Somehow the BPF programs would need to be "marked" at
> > load/verification time with respect to their lockdown requirements so
> > that decisions can be made later. Perhaps a flag in bpf_prog_aux?
> >
> > 2. While it looks like it should be possible to iterate over all of
> > the loaded BPF programs in the LSM notifier callback via
> > idr_for_each(prog_idr, ...), it is not clear to me if it is possible
> > to safely remove, or somehow disable, BPF programs once they have been
> > loaded. Hopefully the BPF folks can help answer that question.
> >
> > 3. Disabling of BPF programs might be preferable to removing them
> > entirely on LSM policy changes as it would be possible to make the
> > lockdown state less restrictive at a future point in time, allowing
> > for the BPF program to be executed again. Once again, not sure if
> > this is even possible.
>
> Part of why this gets really complex/impossible is that BPF programs in
> the kernel are reference counted from various sides, be it that there
> are references from user space to them (fd from application, BPF fs, or
> BPF links), hooks where they are attached to as well as tail call maps
> where one BPF prog calls into another. There is currently also no global
> infra of some sort where you could piggy back to atomically keep track of
> all the references in a list or such. And the other thing is that BPF progs
> have no ownership that is tied to a specific task after they have been
> loaded. Meaning, once they are loaded into the kernel by an application
> and attached to a specific hook, they can remain there potentially until
> reboot of the node, so lifecycle of the user space application != lifecycle
> of the BPF program.
I don't think the disjoint lifecycle or lack of task ownership is a
deal breaker from a LSM perspective as the LSMs can stash whatever
info they need in the security pointer during the program allocation
hook, e.g. selinux_bpf_prog_alloc() saves the security domain which
allocates/loads the BPF program.
The thing I'm worried about would be the case where a LSM policy
change requires that an existing BPF program be removed or disabled.
I'm guessing based on the refcounting that there is not presently a
clean way to remove a BPF program from the system, but is this
something we could resolve? If we can't safely remove a BPF program
from the system, can we replace/swap it with an empty/NULL BPF
program?
> It's maybe best to compare this aspect to kernel modules in the sense that
> you have an application that loads it into the kernel (insmod, etc, where
> you could also enforce lockdown signature check), but after that, they can
> be managed by other entities as well (implicitly refcounted from kernel,
> removed by other applications, etc).
Well, I guess we could consider BPF programs as out-of-tree kernel
modules that potentially do very odd and dangerous things, e.g.
performing access control checks *inside* access control checks ...
but yeah, I get your point at a basic level, I just think that
comparing BPF programs to kernel modules is a not-so-great comparison
in general.
> My understanding of the lockdown settings are that users have options
> to select/enforce a lockdown level of
> CONFIG_LOCK_DOWN_KERNEL_FORCE_{INTEGRITY,
> CONFIDENTIALITY} at compilation time, they have a lockdown={integrity|
> confidentiality} boot-time parameter, /sys/kernel/security/lockdown,
> and then more fine-grained policy via 59438b46471a
> ("securi
Re: [PATCH v2] lockdown,selinux: avoid bogus SELinux lockdown permission checks
On Fri, May 28, 2021 at 2:28 PM Daniel Borkmann wrote:
> In the case of tracing, it's different. You install small programs that are
> triggered when certain events fire. Random example from bpftrace's README [0],
> you want to generate a histogram of syscall counts by program. One-liner is:
>
>bpftrace -e 'tracepoint:raw_syscalls:sys_enter { @[comm] = count(); }'
>
> bpftrace then goes and generates a BPF prog from this internally. One way of
> doing it could be to call bpf_get_current_task() helper and then access
> current->comm via one of bpf_probe_read_kernel{,_str}() helpers ...
I think we can all agree that the BPF tracing is a bit chaotic in the
sense that the tracing programs can be executed in various
places/contexts and that presents some challenges with respect to
access control and auditing. If you are following the io_uring stuff
that is going on now you can see a little of what is required to make
audit work properly in the various io_uring contexts and that is
relatively small compared to what is possible with BPF tracing. Of
course this assumes I've managed to understand bpf tracing properly
this morning, and I very well may still be missing points and/or
confused about some of the important details. Corrections are
welcome.
Daniel's patch side steps that worry by just doing the lockdown
permission check when the BPF program is loaded, but that isn't a
great solution if the policy changes afterward. I was hoping there
might be some way to perform the permission check as needed, but the
more I look the more that appears to be difficult, if not impossible
(once again, corrections are welcome).
I'm now wondering if the right solution here is to make use of the LSM
notifier mechanism. I'm not yet entirely sure if this would work from
a BPF perspective, but I could envision the BPF subsystem registering
a LSM notification callback via register_blocking_lsm_notifier(), see
if Infiniband code as an example, and then when the LSM(s) policy
changes the BPF subsystem would get a notification and it could
revalidate the existing BPF programs and take block/remove/whatever
the offending BPF programs. This obviously requires a few things
which I'm not sure are easily done, or even possible:
1. Somehow the BPF programs would need to be "marked" at
load/verification time with respect to their lockdown requirements so
that decisions can be made later. Perhaps a flag in bpf_prog_aux?
2. While it looks like it should be possible to iterate over all of
the loaded BPF programs in the LSM notifier callback via
idr_for_each(prog_idr, ...), it is not clear to me if it is possible
to safely remove, or somehow disable, BPF programs once they have been
loaded. Hopefully the BPF folks can help answer that question.
3. Disabling of BPF programs might be preferable to removing them
entirely on LSM policy changes as it would be possible to make the
lockdown state less restrictive at a future point in time, allowing
for the BPF program to be executed again. Once again, not sure if
this is even possible.
Related, the lockdown LSM should probably also grow LSM notifier
support similar to selinux_lsm_notifier_avc_callback(), for example
either lock_kernel_down() or lockdown_write() might want to do a
call_blocking_lsm_notifier(LSM_POLICY_CHANGE, NULL) call.
--
paul moore
www.paul-moore.com
Re: [PATCH v2] lockdown,selinux: avoid bogus SELinux lockdown permission checks
On Fri, May 28, 2021 at 2:28 PM Daniel Borkmann wrote:
> On 5/28/21 5:47 PM, Paul Moore wrote:
> > Let's reset.
>
> Sure, yep, lets shortly take one step back. :)
>
> > What task_struct is running the BPF tracing program which is calling
> > into security_locked_down()? My current feeling is that it is this
> > context/domain/cred that should be used for the access control check;
> > in the cases where it is a kernel thread, I think passing NULL is
> > reasonable, but I think the proper thing for SELinux is to interpret
> > NULL as kernel_t.
>
> If this was a typical LSM hook and, say, your app calls into bind(2) where
> we then invoke security_socket_bind() and check 'current' task, then I'm all
> with you, because this was _explicitly initiated_ by the httpd app, so that
> allow/deny policy belongs in the context of httpd.
>
> In the case of tracing, it's different. You install small programs that are
> triggered when certain events fire. Random example from bpftrace's README [0],
> you want to generate a histogram of syscall counts by program. One-liner is:
>
>bpftrace -e 'tracepoint:raw_syscalls:sys_enter { @[comm] = count(); }'
>
> bpftrace then goes and generates a BPF prog from this internally. One way of
> doing it could be to call bpf_get_current_task() helper and then access
> current->comm via one of bpf_probe_read_kernel{,_str}() helpers. So the
> program itself has nothing to do with httpd or any other random app doing
> a syscall here. The BPF prog _explicitly initiated_ the lockdown check.
> The allow/deny policy belongs in the context of bpftrace: meaning, you want
> to grant bpftrace access to use these helpers, but other tracers on the
> systems like my_random_tracer not. While this works for prior mentioned
> cases of security_locked_down() with open_kcore() for /proc/kcore access
> or the module_sig_check(), it is broken for tracing as-is, and the patch
> I sent earlier fixes this.
Sigh.
Generally it's helpful when someone asks a question if you answer it
directly before going off and answering your own questions. Listen, I
get it, you wrote a patch and it fixes your problem (you've mentioned
that already) and it's wonderful and all that, but the rest of us
(maybe just me) need to sort this out too and talking past questions
isn't a great way to help us get there (once again, maybe just me). I
think I can infer an answer from you, but you've made me grumpy now so
I'm not ACK'ing or NACK'ing anything right now; I clearly need to go
spend some time reading through BPF code. Woo.
>[0] https://github.com/iovisor/bpftrace
--
paul moore
www.paul-moore.com
Re: [PATCH v2] lockdown,selinux: avoid bogus SELinux lockdown permission checks
On Fri, May 28, 2021 at 10:43 AM Daniel Borkmann wrote: > On 5/28/21 3:42 PM, Ondrej Mosnacek wrote: > > (I'm off work today and plan to reply also to Paul's comments next > > week, but for now let me at least share a couple quick thoughts on > > Daniel's patch.) Oooh, I sense some disagreement brewing :) > > On Fri, May 28, 2021 at 11:56 AM Daniel Borkmann > > wrote: > >> On 5/28/21 9:09 AM, Daniel Borkmann wrote: > >>> On 5/28/21 3:37 AM, Paul Moore wrote: > >>>> On Mon, May 17, 2021 at 5:22 AM Ondrej Mosnacek > >>>> wrote: ... > >> Ondrej / Paul / Jiri: at least for the BPF tracing case specifically (I > >> haven't looked > >> at the rest but it's also kind of independent), the attached fix should > >> address both > >> reported issues, please take a look & test. > > > > Thanks, I like this solution, although there are a few gotchas: > > > > 1. This patch creates a slight "regression" in that if someone flips > > the Lockdown LSM into lockdown mode on runtime, existing (already > > loaded) BPF programs will still be able to call the > > confidentiality-breaching helpers, while before the lockdown would > > apply also to them. Personally, I don't think it's a big deal (and I > > bet there are other existing cases where some handle kept from before > > lockdown could leak data), but I wanted to mention it in case someone > > thinks the opposite. > > Yes, right, though this is nothing new either in the sense that there are > plenty of other cases with security_locked_down() that operate this way > e.g. take the open_kcore() for /proc/kcore access or the module_sig_check() > for mod signatures just to pick some random ones, same approach where the > enforcement is happen at open/load time. Another, yes, this is not really a good thing to do. Also, just because there are other places that don't really do The Right Thing doesn't mean that it is okay to also not do The Right Thing here. It's basically the two-wrongs-don't-make-a-right issue applied to kernel code. -- paul moore www.paul-moore.com
Re: [PATCH v2] lockdown,selinux: avoid bogus SELinux lockdown permission checks
On Fri, May 28, 2021 at 3:10 AM Daniel Borkmann wrote:
> On 5/28/21 3:37 AM, Paul Moore wrote:
> > On Mon, May 17, 2021 at 5:22 AM Ondrej Mosnacek wrote:
> >>
> >> Commit 59438b46471a ("security,lockdown,selinux: implement SELinux
> >> lockdown") added an implementation of the locked_down LSM hook to
> >> SELinux, with the aim to restrict which domains are allowed to perform
> >> operations that would breach lockdown.
> >>
> >> However, in several places the security_locked_down() hook is called in
> >> situations where the current task isn't doing any action that would
> >> directly breach lockdown, leading to SELinux checks that are basically
> >> bogus.
> >>
> >> Since in most of these situations converting the callers such that
> >> security_locked_down() is called in a context where the current task
> >> would be meaningful for SELinux is impossible or very non-trivial (and
> >> could lead to TOCTOU issues for the classic Lockdown LSM
> >> implementation), fix this by modifying the hook to accept a struct cred
> >> pointer as argument, where NULL will be interpreted as a request for a
> >> "global", task-independent lockdown decision only. Then modify SELinux
> >> to ignore calls with cred == NULL.
> >
> > I'm not overly excited about skipping the access check when cred is
> > NULL. Based on the description and the little bit that I've dug into
> > thus far it looks like using SECINITSID_KERNEL as the subject would be
> > much more appropriate. *Something* (the kernel in most of the
> > relevant cases it looks like) is requesting that a potentially
> > sensitive disclosure be made, and ignoring it seems like the wrong
> > thing to do. Leaving the access control intact also provides a nice
> > avenue to audit these requests should users want to do that.
>
> I think the rationale/workaround for ignoring calls with cred == NULL (or the
> previous
> patch with the unimplemented hook) from Ondrej was two-fold, at least
> speaking for his
> seen tracing cases:
>
>i) The audit events that are triggered due to calls to
> security_locked_down()
> can OOM kill a machine, see below details [0].
>
> ii) It seems to be causing a deadlock via slow_avc_audit() ->
> audit_log_end()
> when presumingly trying to wake up kauditd [1].
>
> How would your suggestion above solve both i) and ii)?
First off, a bit of general commentary - I'm not sure if Ondrej was
aware of this, but info like that is good to have in the commit
description. Perhaps it was in the linked RHBZ but I try not to look
at those when reviewing patches; the commit descriptions must be
self-sufficient since we can't rely on the accessibility or the
lifetime of external references. It's fine if people want to include
external links in their commits, I would actually even encourage it in
some cases, but the links shouldn't replace a proper description of
the problem and why the proposed solution is The Best Solution.
With that out of the way, it sounds like your issue isn't so much the
access check, but rather the frequency of the access denials and the
resulting audit records in your particular use case. My initial
reaction is that you might want to understand why you are getting so
many SELinux access denials, your loaded security policy clearly does
not match with your intended use :) Beyond that, if you want to
basically leave things as-is but quiet the high frequency audit
records that result from these SELinux denials you might want to look
into the SELinux "dontaudit" policy rule, it was created for things
like this. Some info can be found in The SELinux Notebook, relevant
link below:
*
https://github.com/SELinuxProject/selinux-notebook/blob/main/src/avc_rules.md#dontaudit
The deadlock issue that was previously reported remains an open case
as far as I'm concerned; I'm presently occupied trying to sort out a
rather serious issue with respect to io_uring and LSM/audit (plus
general stuff at $DAYJOB) so I haven't had time to investigate this
any further. Of course anyone else is welcome to dive into it (I
always want to encourage this, especially from "performance people"
who just want to shut it all off), however if the answer is basically
"disable LSM and/or audit checks" you have to know that it is going to
result in a high degree of skepticism from me, so heavy documentation
on why it is The Best Solution would be a very good thing :) Beyond
that, I think the suggestions above of "why do you have so many policy
denials?" and "have you looked into dontaudit?" are solid places to
look for a solution in your particular case.
> >> Since most c
Re: [PATCH v2] lockdown,selinux: avoid bogus SELinux lockdown permission checks
er you've read all of the above I hope you can understand why I
disagree with this.
> ... since the eventual leak can be circumvented anyway via b)
I don't follow the statement above ... ? However I'm not sure it
matters much considering my other concerns.
> plus there is no way for the task to indicate that it doesn't care
> about the actual key value, so the check could generate a lot of
> noise.
>
> Improvements-suggested-by: Casey Schaufler
> Fixes: 59438b46471a ("security,lockdown,selinux: implement SELinux lockdown")
> Signed-off-by: Ondrej Mosnacek
> ---
>
> v2:
> - change to a single hook based on suggestions by Casey Schaufler
>
> v1: https://lore.kernel.org/lkml/20210507114048.138933-1-omosn...@redhat.com/
>
> arch/powerpc/xmon/xmon.c | 4 ++--
> fs/tracefs/inode.c| 2 +-
> include/linux/lsm_hook_defs.h | 3 ++-
> include/linux/lsm_hooks.h | 3 ++-
> include/linux/security.h | 11 ---
> kernel/trace/bpf_trace.c | 4 ++--
> net/xfrm/xfrm_user.c | 2 +-
> security/lockdown/lockdown.c | 5 +++--
> security/security.c | 6 +++---
> security/selinux/hooks.c | 12 +---
> 10 files changed, 33 insertions(+), 19 deletions(-)
--
paul moore
www.paul-moore.com
Re: [PATCH v2] lockdown,selinux: avoid bogus SELinux lockdown permission checks
On Thu, May 27, 2021 at 12:33 AM James Morris wrote: > On Wed, 26 May 2021, Ondrej Mosnacek wrote: > > > Thanks, Michael! > > > > James/Paul, is there anything blocking this patch from being merged? > > Especially the BPF case is causing real trouble for people and the > > only workaround is to broadly allow lockdown::confidentiality in the > > policy. > > It would be good to see more signoffs/reviews, especially from Paul, but > he is busy with the io_uring stuff. Yes, it's been a busy week with various things going on around here. I looked at the v1 posting but haven't had a chance yet to look at v2; I promise to get to it today, but it might not happen until later tonight. > Let's see if anyone else can look at this in the next couple of days. -- paul moore www.paul-moore.com
Re: [PATCH v4 2/3] audit: add support for the openat2 syscall
On Thu, May 20, 2021 at 3:58 AM Christian Brauner
wrote:
> On Wed, May 19, 2021 at 04:00:21PM -0400, Richard Guy Briggs wrote:
> > The openat2(2) syscall was added in kernel v5.6 with commit fddb5d430ad9
> > ("open: introduce openat2(2) syscall")
> >
> > Add the openat2(2) syscall to the audit syscall classifier.
> >
> > Link: https://github.com/linux-audit/audit-kernel/issues/67
> > Signed-off-by: Richard Guy Briggs
> > Link:
> > https://lore.kernel.org/r/f5f1a4d8699613f8c02ce762807228c841c2e26f.1621363275.git@redhat.com
> > ---
> > arch/alpha/kernel/audit.c | 2 ++
> > arch/ia64/kernel/audit.c| 2 ++
> > arch/parisc/kernel/audit.c | 2 ++
> > arch/parisc/kernel/compat_audit.c | 2 ++
> > arch/powerpc/kernel/audit.c | 2 ++
> > arch/powerpc/kernel/compat_audit.c | 2 ++
> > arch/s390/kernel/audit.c| 2 ++
> > arch/s390/kernel/compat_audit.c | 2 ++
> > arch/sparc/kernel/audit.c | 2 ++
> > arch/sparc/kernel/compat_audit.c| 2 ++
> > arch/x86/ia32/audit.c | 2 ++
> > arch/x86/kernel/audit_64.c | 2 ++
> > include/linux/auditsc_classmacros.h | 1 +
> > kernel/auditsc.c| 3 +++
> > lib/audit.c | 4
> > lib/compat_audit.c | 4
> > 16 files changed, 36 insertions(+)
...
> > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> > index d775ea16505b..3f59ab209dfd 100644
> > --- a/kernel/auditsc.c
> > +++ b/kernel/auditsc.c
> > @@ -76,6 +76,7 @@
> > #include
> > #include
> > #include
> > +#include
> >
> > #include "audit.h"
> >
> > @@ -196,6 +197,8 @@ static int audit_match_perm(struct audit_context *ctx,
> > int mask)
> > return ((mask & AUDIT_PERM_WRITE) && ctx->argv[0] ==
> > SYS_BIND);
> > case AUDITSC_EXECVE:
> > return mask & AUDIT_PERM_EXEC;
> > + case AUDITSC_OPENAT2:
> > + return mask & ACC_MODE((u32)((struct open_how
> > *)ctx->argv[2])->flags);
>
> That's a lot of dereferncing, casting and masking all at once. Maybe a
> small static inline helper would be good for the sake of legibility? Sm
> like:
>
> static inline u32 audit_openat2_acc(struct open_how *how, int mask)
> {
> u32 flags = how->flags;
> return mask & ACC_MODE(flags);
> }
>
> but not sure. Just seems more legible to me.
> Otherwise.
I'm on the fence about this. I understand Christian's concern, but I
have a bit of hatred towards single caller functions like this. Since
this function isn't really high-touch, and I don't expect that to
change in the near future, let's leave the casting mess as-is.
--
paul moore
www.paul-moore.com
Re: [PATCH v3 1/3] audit: replace magic audit syscall class numbers with macros
On Tue, May 11, 2021 at 1:14 PM Richard Guy Briggs wrote: > > On 2021-05-10 21:23, Paul Moore wrote: > > On Fri, Apr 30, 2021 at 4:36 PM Richard Guy Briggs wrote: > > > > > > Replace audit syscall class magic numbers with macros. > > > > > > This required putting the macros into new header file > > > include/linux/auditscm.h since the syscall macros were included for both > > > 64 > > > bit and 32 bit in any compat code, causing redefinition warnings. > > > > The ifndef/define didn't protect against redeclaration? Huh. Maybe > > I'm not thinking about this correctly, or the arch specific code is > > doing something wonky ... > > > > Regardless, assuming that it is necessary, I would prefer if we called > > it auditsc.h instead of auditscm.h; the latter makes me think of > > sockets and not syscalls. > > The "m" was for "macros", since there are auditsc bits in audit.h as > well, but I have no significant objection. Yes, I figured as much, but my comment about it looking like a socket "thing" still stands. I'm open to other ideas if you don't like auditsc.h, I just don't like auditscm.h. -- paul moore www.paul-moore.com
Re: [PATCH v3 1/3] audit: replace magic audit syscall class numbers with macros
On Fri, Apr 30, 2021 at 4:36 PM Richard Guy Briggs wrote:
>
> Replace audit syscall class magic numbers with macros.
>
> This required putting the macros into new header file
> include/linux/auditscm.h since the syscall macros were included for both 64
> bit and 32 bit in any compat code, causing redefinition warnings.
The ifndef/define didn't protect against redeclaration? Huh. Maybe
I'm not thinking about this correctly, or the arch specific code is
doing something wonky ...
Regardless, assuming that it is necessary, I would prefer if we called
it auditsc.h instead of auditscm.h; the latter makes me think of
sockets and not syscalls.
> Signed-off-by: Richard Guy Briggs
> ---
> MAINTAINERS| 1 +
> arch/alpha/kernel/audit.c | 8
> arch/ia64/kernel/audit.c | 8
> arch/parisc/kernel/audit.c | 8
> arch/parisc/kernel/compat_audit.c | 9 +
> arch/powerpc/kernel/audit.c| 10 +-
> arch/powerpc/kernel/compat_audit.c | 11 ++-
> arch/s390/kernel/audit.c | 10 +-
> arch/s390/kernel/compat_audit.c| 11 ++-
> arch/sparc/kernel/audit.c | 10 +-
> arch/sparc/kernel/compat_audit.c | 11 ++-
> arch/x86/ia32/audit.c | 11 ++-
> arch/x86/kernel/audit_64.c | 8
> include/linux/audit.h | 1 +
> include/linux/auditscm.h | 23 +++
> kernel/auditsc.c | 12 ++--
> lib/audit.c| 10 +-
> lib/compat_audit.c | 11 ++-
> 18 files changed, 102 insertions(+), 71 deletions(-)
> create mode 100644 include/linux/auditscm.h
...
> diff --git a/include/linux/auditscm.h b/include/linux/auditscm.h
> new file mode 100644
> index ..1c4f0ead5931
> --- /dev/null
> +++ b/include/linux/auditscm.h
> @@ -0,0 +1,23 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/* auditscm.h -- Auditing support syscall macros
> + *
> + * Copyright 2021 Red Hat Inc., Durham, North Carolina.
> + * All Rights Reserved.
> + *
> + * Author: Richard Guy Briggs
> + */
> +#ifndef _LINUX_AUDITSCM_H_
> +#define _LINUX_AUDITSCM_H_
> +
> +enum auditsc_class_t {
> + AUDITSC_NATIVE = 0,
> + AUDITSC_COMPAT,
> + AUDITSC_OPEN,
> + AUDITSC_OPENAT,
> + AUDITSC_SOCKETCALL,
> + AUDITSC_EXECVE,
> +
> + AUDITSC_NVALS /* count */
> +};
> +
> +#endif
--
paul moore
www.paul-moore.com
Re: [PATCH v2 13/13] syscall_get_arch: add "struct task_struct *" argument
On Sun, Mar 17, 2019 at 7:30 PM Dmitry V. Levin wrote:
>
> This argument is required to extend the generic ptrace API with
> PTRACE_GET_SYSCALL_INFO request: syscall_get_arch() is going
> to be called from ptrace_request() along with syscall_get_nr(),
> syscall_get_arguments(), syscall_get_error(), and
> syscall_get_return_value() functions with a tracee as their argument.
>
> The primary intent is that the triple (audit_arch, syscall_nr, arg1..arg6)
> should describe what system call is being called and what its arguments
> are.
>
> Reverts: 5e937a9ae913 ("syscall_get_arch: remove useless function arguments")
> Reverts: 1002d94d3076 ("syscall.h: fix doc text for syscall_get_arch()")
> Reviewed-by: Andy Lutomirski # for x86
> Reviewed-by: Palmer Dabbelt
> Acked-by: Paul Moore
> Acked-by: Paul Burton # MIPS parts
> Acked-by: Michael Ellerman (powerpc)
> Acked-by: Kees Cook # seccomp parts
> Acked-by: Mark Salter # for the c6x bit
> Cc: Elvira Khabirova
> Cc: Eugene Syromyatnikov
> Cc: Oleg Nesterov
> Cc: x...@kernel.org
> Cc: linux-al...@vger.kernel.org
> Cc: linux-snps-...@lists.infradead.org
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: linux-c6x-...@linux-c6x.org
> Cc: uclinux-h8-de...@lists.sourceforge.jp
> Cc: linux-hexa...@vger.kernel.org
> Cc: linux-i...@vger.kernel.org
> Cc: linux-m...@lists.linux-m68k.org
> Cc: linux-m...@vger.kernel.org
> Cc: nios2-...@lists.rocketboards.org
> Cc: openr...@lists.librecores.org
> Cc: linux-par...@vger.kernel.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux-ri...@lists.infradead.org
> Cc: linux-s...@vger.kernel.org
> Cc: linux...@vger.kernel.org
> Cc: sparcli...@vger.kernel.org
> Cc: linux...@lists.infradead.org
> Cc: linux-xte...@linux-xtensa.org
> Cc: linux-a...@vger.kernel.org
> Cc: linux-au...@redhat.com
> Signed-off-by: Dmitry V. Levin
> ---
>
> Notes:
> v2: unchanged
>
> arch/alpha/include/asm/syscall.h | 2 +-
> arch/arc/include/asm/syscall.h| 2 +-
> arch/arm/include/asm/syscall.h| 2 +-
> arch/arm64/include/asm/syscall.h | 4 ++--
> arch/c6x/include/asm/syscall.h| 2 +-
> arch/csky/include/asm/syscall.h | 2 +-
> arch/h8300/include/asm/syscall.h | 2 +-
> arch/hexagon/include/asm/syscall.h| 2 +-
> arch/ia64/include/asm/syscall.h | 2 +-
> arch/m68k/include/asm/syscall.h | 2 +-
> arch/microblaze/include/asm/syscall.h | 2 +-
> arch/mips/include/asm/syscall.h | 6 +++---
> arch/mips/kernel/ptrace.c | 2 +-
> arch/nds32/include/asm/syscall.h | 2 +-
> arch/nios2/include/asm/syscall.h | 2 +-
> arch/openrisc/include/asm/syscall.h | 2 +-
> arch/parisc/include/asm/syscall.h | 4 ++--
> arch/powerpc/include/asm/syscall.h| 10 --
> arch/riscv/include/asm/syscall.h | 2 +-
> arch/s390/include/asm/syscall.h | 4 ++--
> arch/sh/include/asm/syscall_32.h | 2 +-
> arch/sh/include/asm/syscall_64.h | 2 +-
> arch/sparc/include/asm/syscall.h | 5 +++--
> arch/unicore32/include/asm/syscall.h | 2 +-
> arch/x86/include/asm/syscall.h| 8 +---
> arch/x86/um/asm/syscall.h | 2 +-
> arch/xtensa/include/asm/syscall.h | 2 +-
> include/asm-generic/syscall.h | 5 +++--
> kernel/auditsc.c | 4 ++--
> kernel/seccomp.c | 4 ++--
> 30 files changed, 52 insertions(+), 42 deletions(-)
Merged into audit/next, thanks everyone.
--
paul moore
www.paul-moore.com
Re: [QUESTION] powerpc, libseccomp, and spu
On Tue, Feb 12, 2019 at 9:50 AM Tom Hromatka wrote: > On 2/11/19 11:54 AM, Tom Hromatka wrote: > > PowerPC experts, > > > > Paul Moore and I are working on the v2.4 release of libseccomp, > > and as part of this work I need to update the syscall table for > > each architecture. > > > > I have incorporated the new ppc syscall.tbl into libseccomp, but > > I am not familiar with the value of "spu" in the ABI column. For > > example: > > > > 2232umountsys_oldumount > > 2264umountsys_ni_syscall > > 22spuumountsys_ni_syscall > > > > In libseccomp, we maintain a 32-bit ppc syscall table and a 64-bit > > ppc syscall table. Do we also need to add a "spu" ppc syscall > > table? Some clarification on the syscalls marked "spu" and "nospu" > > would be greatly appreciated. > > Thanks for the awesome responses, Ben and Michael. I'll definitely > get Paul's input as well, but it sounds reasonable to exclude SPUs > from the newest libseccomp release. Based on this thread, I don't think we need to worry about "spu" at this point in time. Thanks everyone. > Michael's recommendation to replace "nospu" with common" and ignore > "spu" entirely has allowed ppc and ppc64 to pass all of our internal > checks. > > Thanks again! > > Tom -- paul moore www.paul-moore.com
Re: [PATCH v6 00/27] ptrace: add PTRACE_GET_SYSCALL_INFO request
em from
> ptrace_syscall_info.{entry,seccomp} substructures to ptrace_syscall_info
> and initializing them for all stops.
> * Add PTRACE_SYSCALL_INFO_NONE, set it when not in a syscall stop,
> so e.g. "strace -i" could use PTRACE_SYSCALL_INFO_SECCOMP to obtain
> instruction_pointer when the tracee is in a signal stop.
> * Patch all remaining architectures to provide all necessary
> syscall_get_* functions.
> * Make available for all architectures: do not conditionalize on
> CONFIG_HAVE_ARCH_TRACEHOOK since all syscall_get_* functions
> are implemented on all architectures.
> * Add a test for PTRACE_GET_SYSCALL_INFO to selftests/ptrace.
>
> v4:
> * Do not introduce task_struct.ptrace_event,
> use child->last_siginfo->si_code instead.
> * Implement PTRACE_SYSCALL_INFO_SECCOMP and ptrace_syscall_info.seccomp
> support along with PTRACE_SYSCALL_INFO_{ENTRY,EXIT} and
> ptrace_syscall_info.{entry,exit}.
>
> v3:
> * Change struct ptrace_syscall_info.
> * Support PTRACE_EVENT_SECCOMP by adding ptrace_event to task_struct.
> * Add proper defines for ptrace_syscall_info.op values.
> * Rename PT_SYSCALL_IS_ENTERING and PT_SYSCALL_IS_EXITING to
> PTRACE_EVENTMSG_SYSCALL_ENTRY and PTRACE_EVENTMSG_SYSCALL_EXIT
> * and move them to uapi.
>
> v2:
> * Do not use task->ptrace.
> * Replace entry_info.is_compat with entry_info.arch, use
> syscall_get_arch().
> * Use addr argument of sys_ptrace to get expected size of the struct;
> return full size of the struct.
>
> Dmitry V. Levin (25):
> asm-generic/syscall.h: prepare for inclusion by other files
> asm-generic/syscall.h: turn syscall_[gs]et_arguments into wrappers
> alpha: define remaining syscall_get_* functions
> Move EM_ARCOMPACT and EM_ARCV2 to uapi/linux/elf-em.h
> arc: define syscall_get_arch()
> c6x: define syscall_get_arch()
> elf-em.h: add EM_CSKY
> csky: define syscall_get_arch()
> h8300: define remaining syscall_get_* functions
> Move EM_HEXAGON to uapi/linux/elf-em.h
> hexagon: define remaining syscall_get_* functions
> Move EM_NDS32 to uapi/linux/elf-em.h
> nds32: define syscall_get_arch()
> nios2: define syscall_get_arch()
> m68k: add asm/syscall.h
> mips: define syscall_get_error()
> parisc: define syscall_get_error()
> powerpc: define syscall_get_error()
> riscv: define syscall_get_arch()
> Move EM_XTENSA to uapi/linux/elf-em.h
> xtensa: define syscall_get_* functions
> Move EM_UNICORE to uapi/linux/elf-em.h
> unicore32: add asm/syscall.h
> syscall_get_arch: add "struct task_struct *" argument
> selftests/ptrace: add a test case for PTRACE_GET_SYSCALL_INFO
>
> Elvira Khabirova (2):
> powerpc/ptrace: replace ptrace_report_syscall() with a tracehook call
> ptrace: add PTRACE_GET_SYSCALL_INFO request
As mentioned in the previous patchsets, this all looks fine to me from
an audit perspective (although there is not much audit code to really
comment on). Feel free to add my ACK to the audit related patches.
Acked-by: Paul Moore
> arch/alpha/include/asm/syscall.h | 31 +-
> arch/arc/include/asm/elf.h| 6 +-
> arch/arc/include/asm/syscall.h| 11 +
> arch/arm/include/asm/syscall.h| 2 +-
> arch/arm64/include/asm/syscall.h | 4 +-
> arch/c6x/include/asm/syscall.h| 7 +
> arch/csky/include/asm/syscall.h | 7 +
> arch/h8300/include/asm/syscall.h | 19 ++
> arch/hexagon/include/asm/elf.h| 6 +-
> arch/hexagon/include/asm/syscall.h| 22 ++
> arch/ia64/include/asm/syscall.h | 2 +-
> arch/m68k/include/asm/syscall.h | 42 +++
> arch/microblaze/include/asm/syscall.h | 2 +-
> arch/mips/include/asm/syscall.h | 12 +-
> arch/mips/kernel/ptrace.c | 2 +-
> arch/nds32/include/asm/elf.h | 3 +-
> arch/nds32/include/asm/syscall.h | 8 +
> arch/nios2/include/asm/syscall.h | 6 +
> arch/openrisc/include/asm/syscall.h | 2 +-
> arch/parisc/include/asm/syscall.h | 11 +-
> arch/powerpc/include/asm/syscall.h| 20 +-
> arch/powerpc/kernel/ptrace.c | 7 +-
> arch/riscv/include/asm/syscall.h | 10 +
> arch/s390/include/asm/syscall.h | 4 +-
> arch/sh/include/asm/syscall_32.h | 2 +-
> arch/sh/include/asm/syscall_64.h | 2 +-
> arch/sparc/include/asm/syscall.h | 5 +-
>
Re: [PATCH] [RFC v2] Drop all 00-INDEX files from Documentation/
On Mon, Sep 3, 2018 at 6:15 PM Henrik Austad wrote: > This is a respin with a wider audience (all that get_maintainer returned) > and I know this spams a *lot* of people. Not sure what would be the correct > way, so my apologies for ruining your inbox. > > The 00-INDEX files are supposed to give a summary of all files present > in a directory, but these files are horribly out of date and their > usefulness is brought into question. Often a simple "ls" would reveal > the same information as the filenames are generally quite descriptive as > a short introduction to what the file covers (it should not surprise > anyone what Documentation/sched/sched-design-CFS.txt covers) > > A few years back it was mentioned that these files were no longer really > needed, and they have since then grown further out of date, so perhaps > it is time to just throw them out. > > A short status yields the following _outdated_ 00-INDEX files, first > counter is files listed in 00-INDEX but missing in the directory, last > is files present but not listed in 00-INDEX. > > List of outdated 00-INDEX: > Documentation: (4/10) > Documentation/sysctl: (0/1) > Documentation/timers: (1/0) > Documentation/blockdev: (3/1) > Documentation/w1/slaves: (0/1) > Documentation/locking: (0/1) > Documentation/devicetree: (0/5) > Documentation/power: (1/1) > Documentation/powerpc: (0/5) > Documentation/arm: (1/0) > Documentation/x86: (0/9) > Documentation/x86/x86_64: (1/1) > Documentation/scsi: (4/4) > Documentation/filesystems: (2/9) > Documentation/filesystems/nfs: (0/2) > Documentation/cgroup-v1: (0/2) > Documentation/kbuild: (0/4) > Documentation/spi: (1/0) > Documentation/virtual/kvm: (1/0) > Documentation/scheduler: (0/2) > Documentation/fb: (0/1) > Documentation/block: (0/1) > Documentation/networking: (6/37) > Documentation/vm: (1/3) > > Then there are 364 subdirectories in Documentation/ with several files that > are missing 00-INDEX alltogether (and another 120 with a single file and no > 00-INDEX). > > I don't really have an opinion to whether or not we /should/ have 00-INDEX, > but the above 00-INDEX should either be removed or be kept up to date. If > we should keep the files, I can try to keep them updated, but I rather not > if we just want to delete them anyway. > > As a starting point, remove all index-files and references to 00-INDEX and > see where the discussion is going. > > Again, sorry for the insanely wide distribution. > > Signed-off-by: Henrik Austad ... > Signed-off-by: Henrik Austad > --- > Documentation/00-INDEX | 428 > ... Looks reasonable to me, you can add my ACK for the NetLabel bits. Acked-by: Paul Moore -- paul moore www.paul-moore.com
Re: [trivial PATCH] treewide: Align function definition open/close braces
On Sun, Dec 17, 2017 at 7:28 PM, Joe Perches <j...@perches.com> wrote: > Some functions definitions have either the initial open brace and/or > the closing brace outside of column 1. > > Move those braces to column 1. > > This allows various function analyzers like gnu complexity to work > properly for these modified functions. > > Miscellanea: > > o Remove extra trailing ; and blank line from xfs_agf_verify > > Signed-off-by: Joe Perches <j...@perches.com> > --- > git diff -w shows no difference other than the above 'Miscellanea' > > (this is against -next, but it applies against Linus' tree > with a couple offsets) > > arch/x86/include/asm/atomic64_32.h | 2 +- > drivers/acpi/custom_method.c | 2 +- > drivers/acpi/fan.c | 2 +- > drivers/gpu/drm/amd/display/dc/core/dc.c | 2 +- > drivers/media/i2c/msp3400-kthreads.c | 2 +- > drivers/message/fusion/mptsas.c | 2 +- > drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c | 2 +- > drivers/net/wireless/ath/ath9k/xmit.c| 2 +- > drivers/platform/x86/eeepc-laptop.c | 2 +- > drivers/rtc/rtc-ab-b5ze-s3.c | 2 +- > drivers/scsi/dpt_i2o.c | 2 +- > drivers/scsi/sym53c8xx_2/sym_glue.c | 2 +- > fs/locks.c | 2 +- > fs/ocfs2/stack_user.c| 2 +- > fs/xfs/libxfs/xfs_alloc.c| 5 ++--- > fs/xfs/xfs_export.c | 2 +- > kernel/audit.c | 6 +++--- > kernel/trace/trace_printk.c | 4 ++-- > lib/raid6/sse2.c | 14 +++--- > sound/soc/fsl/fsl_dma.c | 2 +- > 20 files changed, 30 insertions(+), 31 deletions(-) For the audit bits ... Acked-by: Paul Moore <p...@paul-moore.com> -- paul moore www.paul-moore.com
Re: [PATCH V4] powerpc: add little endian flag to syscall_get_arch()
On Tuesday, December 09, 2014 03:37:07 PM Richard Guy Briggs wrote:
Since both ppc and ppc64 have LE variants which are now reported by uname,
add that flag (__AUDIT_ARCH_LE) to syscall_get_arch() and add
AUDIT_ARCH_PPC64LE variant.
Without this, perf trace and auditctl fail.
Mainline kernel reports ppc64le (per a058801) but there is no matching
AUDIT_ARCH_PPC64LE.
Since 32-bit PPC LE is not supported by audit, don't advertise it in
AUDIT_ARCH_PPC* variants.
See:
https://www.redhat.com/archives/linux-audit/2014-August/msg00082.html
https://www.redhat.com/archives/linux-audit/2014-December/msg4.html
Signed-off-by: Richard Guy Briggs r...@redhat.com
---
arch/powerpc/include/asm/syscall.h |6 +-
include/uapi/linux/audit.h |2 ++
2 files changed, 7 insertions(+), 1 deletions(-)
The audit changes look fine to me, but as I mentioned earlier, this should go
in via the ppc tree and not the audit tree.
Acked-by: Paul Moore p...@paul-moore.com
diff --git a/arch/powerpc/include/asm/syscall.h
b/arch/powerpc/include/asm/syscall.h index 6fa2708..d1934e5 100644
--- a/arch/powerpc/include/asm/syscall.h
+++ b/arch/powerpc/include/asm/syscall.h
@@ -90,6 +90,10 @@ static inline void syscall_set_arguments(struct
task_struct *task,
static inline int syscall_get_arch(void)
{
- return is_32bit_task() ? AUDIT_ARCH_PPC : AUDIT_ARCH_PPC64;
+ int arch = is_32bit_task() ? AUDIT_ARCH_PPC : AUDIT_ARCH_PPC64;
+#ifdef __LITTLE_ENDIAN__
+ arch |= __AUDIT_ARCH_LE;
+#endif
+ return arch;
}
#endif /* _ASM_SYSCALL_H */
diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index 4d100c8..d82beec 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -364,7 +364,9 @@ enum {
#define AUDIT_ARCH_PARISC(EM_PARISC)
#define AUDIT_ARCH_PARISC64 (EM_PARISC|__AUDIT_ARCH_64BIT)
#define AUDIT_ARCH_PPC (EM_PPC)
+/* do not define AUDIT_ARCH_PPCLE since it is not supported by audit */
#define AUDIT_ARCH_PPC64 (EM_PPC64|__AUDIT_ARCH_64BIT)
+#define AUDIT_ARCH_PPC64LE (EM_PPC64|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
#define AUDIT_ARCH_S390 (EM_S390)
#define AUDIT_ARCH_S390X (EM_S390|__AUDIT_ARCH_64BIT)
#define AUDIT_ARCH_SH(EM_SH)
--
paul moore
security and virtualization @ redhat
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH V3] powerpc: add little endian flag to syscall_get_arch()
On Monday, December 08, 2014 12:59:32 PM Richard Guy Briggs wrote:
Since both ppc and ppc64 have LE variants which are now reported by uname,
add that flag (__AUDIT_ARCH_LE) to syscall_get_arch() and add
AUDIT_ARCH_PPC*LE variants.
Without this, perf trace and auditctl fail.
Mainline kernel reports ppc64le (per a058801) but there is no matching
AUDIT_ARCH_PPC64LE.
Since 32-bit PPC LE is not supported, throw a compiler error rather than
return a bogus architecture to audit.
See:
https://www.redhat.com/archives/linux-audit/2014-August/msg00082.html
https://www.redhat.com/archives/linux-audit/2014-December/msg4.html
v2 - v3:
Throw a compiler error on 32-bit LE.
v1 - v2:
Added ; at the end of the #ifdef-protected line so it actually
compiles
Signed-off-by: Richard Guy Briggs r...@redhat.com
---
arch/powerpc/include/asm/syscall.h |7 +++
include/uapi/linux/audit.h |1 +
2 files changed, 8 insertions(+), 0 deletions(-)
Looks reasonable to me from an audit perspective, but I'll let the ppc folks
merge this patch into their tree.
Acked-by: Paul Moore p...@paul-moore.com
diff --git a/arch/powerpc/include/asm/syscall.h
b/arch/powerpc/include/asm/syscall.h index 6fa2708..cf7fcab 100644
--- a/arch/powerpc/include/asm/syscall.h
+++ b/arch/powerpc/include/asm/syscall.h
@@ -90,6 +90,13 @@ static inline void syscall_set_arguments(struct
task_struct *task,
static inline int syscall_get_arch(void)
{
+#ifdef __LITTLE_ENDIAN__
+ return AUDIT_ARCH_PPC64LE;
+#ifndef CONFIG_64BIT
+#error PPC 32-bit Little Endian architecture not supported.
+#endif /* CONFIG_64BIT */
+#else /* __LITTLE_ENDIAN__ */
return is_32bit_task() ? AUDIT_ARCH_PPC : AUDIT_ARCH_PPC64;
+#endif /* __LITTLE_ENDIAN__ */
}
#endif /* _ASM_SYSCALL_H */
diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index 4d100c8..fa2a6af 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -365,6 +365,7 @@ enum {
#define AUDIT_ARCH_PARISC64 (EM_PARISC|__AUDIT_ARCH_64BIT)
#define AUDIT_ARCH_PPC (EM_PPC)
#define AUDIT_ARCH_PPC64 (EM_PPC64|__AUDIT_ARCH_64BIT)
+#define AUDIT_ARCH_PPC64LE (EM_PPC64|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
#define AUDIT_ARCH_S390 (EM_S390)
#define AUDIT_ARCH_S390X (EM_S390|__AUDIT_ARCH_64BIT)
#define AUDIT_ARCH_SH(EM_SH)
--
paul moore
www.paul-moore.com
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc: add little endian flag to syscall_get_arch()
On Tuesday, December 02, 2014 01:54:16 PM Tony Jones wrote:
On 12/02/2014 01:27 PM, Richard Guy Briggs wrote:
Since both ppc and ppc64 have LE variants which are now reported by uname,
add that flag (__AUDIT_ARCH_LE) to syscall_get_arch() and add
AUDIT_ARCH_PPC*LE variants.
Without this, perf trace and auditctl fail.
Mainline kernel reports ppc64le (per a058801) but there is no matching
AUDIT_ARCH_PPC64LE.
See:
https://www.redhat.com/archives/linux-audit/2014-August/msg00082.html
https://www.redhat.com/archives/linux-audit/2014-December/msg4.html
Signed-off-by: Richard Guy Briggs r...@redhat.com
---
arch/powerpc/include/asm/syscall.h |6 +-
include/uapi/linux/audit.h |2 ++
2 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/arch/powerpc/include/asm/syscall.h
b/arch/powerpc/include/asm/syscall.h index 6fa2708..a58acab 100644
--- a/arch/powerpc/include/asm/syscall.h
+++ b/arch/powerpc/include/asm/syscall.h
@@ -90,6 +90,10 @@ static inline void syscall_set_arguments(struct
task_struct *task,
static inline int syscall_get_arch(void)
{
- return is_32bit_task() ? AUDIT_ARCH_PPC : AUDIT_ARCH_PPC64;
+ int arch = is_32bit_task() ? AUDIT_ARCH_PPC : AUDIT_ARCH_PPC64;
+#ifdef __LITTLE_ENDIAN__
+ arch |= __AUDIT_ARCH_LE
+#endif
+ return arch;
}
#endif /* _ASM_SYSCALL_H */
diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index 4d100c8..fe29a99 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -364,7 +364,9 @@ enum {
#define AUDIT_ARCH_PARISC (EM_PARISC)
#define AUDIT_ARCH_PARISC64(EM_PARISC|__AUDIT_ARCH_64BIT)
#define AUDIT_ARCH_PPC (EM_PPC)
+#define AUDIT_ARCH_PPCLE (EM_PPC|__AUDIT_ARCH_LE)
#define AUDIT_ARCH_PPC64 (EM_PPC64|__AUDIT_ARCH_64BIT)
+#define AUDIT_ARCH_PPC64LE (EM_PPC64|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
#define AUDIT_ARCH_S390(EM_S390)
#define AUDIT_ARCH_S390X (EM_S390|__AUDIT_ARCH_64BIT)
#define AUDIT_ARCH_SH (EM_SH)
IBM would know for certain but I wasn't aware there was a PPCLE (32bit
compat).
FWIW, I've heard the same thing from IBM folks off-list.
--
paul moore
security and virtualization @ redhat
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
