| Message ID | 20220926131643.146502-2-nathanl@linux.ibm.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | powerpc/pseries: restrict error injection and DT changes when locked down | expand |
On Mon, Sep 26, 2022 at 9:17 AM Nathan Lynch <nathanl@linux.ibm.com> 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 <nathanl@linux.ibm.com> > --- > 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 <paul@paul-moore.com> (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 <linux/kernel.h> > #include <linux/notifier.h> > #include <linux/proc_fs.h> > +#include <linux/security.h> > #include <linux/slab.h> > #include <linux/of.h> > > @@ -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 >
On Mon, 2022-09-26 at 08:16 -0500, 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 <nathanl@linux.ibm.com> Seems sensible to me. Reviewed-by: Andrew Donnellan <ajd@linux.ibm.com> > --- > arch/powerpc/platforms/pseries/reconfig.c | 5 +++++ > include/linux/security.h | 1 + > security/security.c | 1 + > 3 files changed, 7 insertions(+) > > 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 <linux/kernel.h> > #include <linux/notifier.h> > #include <linux/proc_fs.h> > +#include <linux/security.h> > #include <linux/slab.h> > #include <linux/of.h> > > @@ -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",
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 <linux/kernel.h> #include <linux/notifier.h> #include <linux/proc_fs.h> +#include <linux/security.h> #include <linux/slab.h> #include <linux/of.h> @@ -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",
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 <nathanl@linux.ibm.com> --- arch/powerpc/platforms/pseries/reconfig.c | 5 +++++ include/linux/security.h | 1 + security/security.c | 1 + 3 files changed, 7 insertions(+)