| Message ID | 20200619124833.633575-5-seth.forshee@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | Lockdown updates | expand |
On Fri, Jun 19, 2020 at 07:48:31AM -0500, Seth Forshee wrote: > From: "Christopher M. Riedl" <cmr@informatik.wtf> > > BugLink: https://bugs.launchpad.net/bugs/1884159 > > Xmon should be either fully or partially disabled depending on the > kernel lockdown state. > > Put xmon into read-only mode for lockdown=integrity and prevent user > entry into xmon when lockdown=confidentiality. Xmon checks the lockdown > state on every attempted entry: > > (1) during early xmon'ing > > (2) when triggered via sysrq > > (3) when toggled via debugfs > > (4) when triggered via a previously enabled breakpoint > > The following lockdown state transitions are handled: > > (1) lockdown=none -> lockdown=integrity > set xmon read-only mode > > (2) lockdown=none -> lockdown=confidentiality > clear all breakpoints, set xmon read-only mode, > prevent user re-entry into xmon > > (3) lockdown=integrity -> lockdown=confidentiality > clear all breakpoints, set xmon read-only mode, > prevent user re-entry into xmon > > Suggested-by: Andrew Donnellan <ajd@linux.ibm.com> > Signed-off-by: Christopher M. Riedl <cmr@informatik.wtf> > Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> > Link: https://lore.kernel.org/r/20190907061124.1947-3-cmr@informatik.wtf > (backported from commit 69393cb03ccdf29f3b452d3482ef918469d1c098) > Signed-off-by: Seth Forshee <seth.forshee@canonical.com> > --- > arch/powerpc/xmon/xmon.c | 106 ++++++++++++++++++++++++++++++++------- > 1 file changed, 89 insertions(+), 17 deletions(-) I was finally able to test this and then noticed that CONFIG_LOCK_DOWN_KERNEL is not set for ppc64el. Should we enable it for this patchset? Cascardo.
On Tue, Jun 23, 2020 at 02:15:00PM -0300, Thadeu Lima de Souza Cascardo wrote: > On Fri, Jun 19, 2020 at 07:48:31AM -0500, Seth Forshee wrote: > > From: "Christopher M. Riedl" <cmr@informatik.wtf> > > > > BugLink: https://bugs.launchpad.net/bugs/1884159 > > > > Xmon should be either fully or partially disabled depending on the > > kernel lockdown state. > > > > Put xmon into read-only mode for lockdown=integrity and prevent user > > entry into xmon when lockdown=confidentiality. Xmon checks the lockdown > > state on every attempted entry: > > > > (1) during early xmon'ing > > > > (2) when triggered via sysrq > > > > (3) when toggled via debugfs > > > > (4) when triggered via a previously enabled breakpoint > > > > The following lockdown state transitions are handled: > > > > (1) lockdown=none -> lockdown=integrity > > set xmon read-only mode > > > > (2) lockdown=none -> lockdown=confidentiality > > clear all breakpoints, set xmon read-only mode, > > prevent user re-entry into xmon > > > > (3) lockdown=integrity -> lockdown=confidentiality > > clear all breakpoints, set xmon read-only mode, > > prevent user re-entry into xmon > > > > Suggested-by: Andrew Donnellan <ajd@linux.ibm.com> > > Signed-off-by: Christopher M. Riedl <cmr@informatik.wtf> > > Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> > > Link: https://lore.kernel.org/r/20190907061124.1947-3-cmr@informatik.wtf > > (backported from commit 69393cb03ccdf29f3b452d3482ef918469d1c098) > > Signed-off-by: Seth Forshee <seth.forshee@canonical.com> > > --- > > arch/powerpc/xmon/xmon.c | 106 ++++++++++++++++++++++++++++++++------- > > 1 file changed, 89 insertions(+), 17 deletions(-) > > I was finally able to test this and then noticed that CONFIG_LOCK_DOWN_KERNEL > is not set for ppc64el. Should we enable it for this patchset? Hmm, I knew that we only started automatically enabling lockdown under secure boot recently for ppc64el, but I didn't notice that the option was turned off. Looks like that's the case until focal. I think whether to turn on lockdown for ppc64el is a sepearte issue from these updates, so let's not do that here. And in that case the xmon patches aren't needed right now.
On Tue, Jun 23, 2020 at 01:30:16PM -0500, Seth Forshee wrote: > On Tue, Jun 23, 2020 at 02:15:00PM -0300, Thadeu Lima de Souza Cascardo wrote: > > On Fri, Jun 19, 2020 at 07:48:31AM -0500, Seth Forshee wrote: > > > From: "Christopher M. Riedl" <cmr@informatik.wtf> > > > > > > BugLink: https://bugs.launchpad.net/bugs/1884159 > > > > > > Xmon should be either fully or partially disabled depending on the > > > kernel lockdown state. > > > > > > Put xmon into read-only mode for lockdown=integrity and prevent user > > > entry into xmon when lockdown=confidentiality. Xmon checks the lockdown > > > state on every attempted entry: > > > > > > (1) during early xmon'ing > > > > > > (2) when triggered via sysrq > > > > > > (3) when toggled via debugfs > > > > > > (4) when triggered via a previously enabled breakpoint > > > > > > The following lockdown state transitions are handled: > > > > > > (1) lockdown=none -> lockdown=integrity > > > set xmon read-only mode > > > > > > (2) lockdown=none -> lockdown=confidentiality > > > clear all breakpoints, set xmon read-only mode, > > > prevent user re-entry into xmon > > > > > > (3) lockdown=integrity -> lockdown=confidentiality > > > clear all breakpoints, set xmon read-only mode, > > > prevent user re-entry into xmon > > > > > > Suggested-by: Andrew Donnellan <ajd@linux.ibm.com> > > > Signed-off-by: Christopher M. Riedl <cmr@informatik.wtf> > > > Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> > > > Link: https://lore.kernel.org/r/20190907061124.1947-3-cmr@informatik.wtf > > > (backported from commit 69393cb03ccdf29f3b452d3482ef918469d1c098) > > > Signed-off-by: Seth Forshee <seth.forshee@canonical.com> > > > --- > > > arch/powerpc/xmon/xmon.c | 106 ++++++++++++++++++++++++++++++++------- > > > 1 file changed, 89 insertions(+), 17 deletions(-) > > > > I was finally able to test this and then noticed that CONFIG_LOCK_DOWN_KERNEL > > is not set for ppc64el. Should we enable it for this patchset? > > Hmm, I knew that we only started automatically enabling lockdown under > secure boot recently for ppc64el, but I didn't notice that the option > was turned off. Looks like that's the case until focal. > > I think whether to turn on lockdown for ppc64el is a sepearte issue from > these updates, so let's not do that here. And in that case the xmon > patches aren't needed right now. Agreed to split the issues. I just couldn't test lockdown and xmon interaction, besides that read-only worked and was turned on by default in this case. But, I could not turn on lockdown, and so could easily override xmon to rw. Cascardo.
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c index 6d93367d600b..db036bf7045a 100644 --- a/arch/powerpc/xmon/xmon.c +++ b/arch/powerpc/xmon/xmon.c @@ -195,6 +195,8 @@ static void dump_tlb_44x(void); static void dump_tlb_book3e(void); #endif +static void clear_all_bpt(void); + #ifdef CONFIG_PPC64 #define REG "%.16lx" #else @@ -290,10 +292,26 @@ Commands:\n\ " U show uptime information\n" " ? help\n" " # n limit output to n lines per page (for dp, dpa, dl)\n" -" zr reboot\n\ - zh halt\n" +" zr reboot\n" +" zh halt\n" ; +static bool xmon_is_locked_down(void) +{ + /* + * Upstream has an integrity level of lockdown and a confidentiality + * level, and xmon_is_locked_down() checks both to determine what + * level of xmon restriction to enforce. For the Ubuntu backport we + * don't have this dual-level approach, and we only need to enforce + * the integrity level. This makes xmon read-only but returns 'false' + * from xmon_is_locked_down(). + */ + if (!xmon_is_ro) + xmon_is_ro = kernel_is_locked_down("xmon write access"); + + return false; +} + static struct pt_regs *xmon_regs; static inline void sync(void) @@ -445,7 +463,10 @@ static bool wait_for_other_cpus(int ncpus) return false; } -#endif /* CONFIG_SMP */ +#else /* CONFIG_SMP */ +static inline void get_output_lock(void) {} +static inline void release_output_lock(void) {} +#endif static inline int unrecoverable_excp(struct pt_regs *regs) { @@ -462,6 +483,7 @@ static int xmon_core(struct pt_regs *regs, int fromipi) int cmd = 0; struct bpt *bp; long recurse_jmp[JMP_BUF_LEN]; + bool locked_down; unsigned long offset; unsigned long flags; #ifdef CONFIG_SMP @@ -472,6 +494,8 @@ static int xmon_core(struct pt_regs *regs, int fromipi) local_irq_save(flags); hard_irq_disable(); + locked_down = xmon_is_locked_down(); + if (!fromipi) { tracing_enabled = tracing_is_on(); tracing_off(); @@ -525,7 +549,8 @@ static int xmon_core(struct pt_regs *regs, int fromipi) if (!fromipi) { get_output_lock(); - excprint(regs); + if (!locked_down) + excprint(regs); if (bp) { printf("cpu 0x%x stopped at breakpoint 0x%lx (", cpu, BP_NUM(bp)); @@ -577,10 +602,14 @@ static int xmon_core(struct pt_regs *regs, int fromipi) } remove_bpts(); disable_surveillance(); - /* for breakpoint or single step, print the current instr. */ - if (bp || TRAP(regs) == 0xd00) - ppc_inst_dump(regs->nip, 1, 0); - printf("enter ? for help\n"); + + if (!locked_down) { + /* for breakpoint or single step, print curr insn */ + if (bp || TRAP(regs) == 0xd00) + ppc_inst_dump(regs->nip, 1, 0); + printf("enter ? for help\n"); + } + mb(); xmon_gate = 1; barrier(); @@ -604,8 +633,9 @@ static int xmon_core(struct pt_regs *regs, int fromipi) spin_cpu_relax(); touch_nmi_watchdog(); } else { - cmd = cmds(regs); - if (cmd != 0) { + if (!locked_down) + cmd = cmds(regs); + if (locked_down || cmd != 0) { /* exiting xmon */ insert_bpts(); xmon_gate = 0; @@ -642,13 +672,16 @@ static int xmon_core(struct pt_regs *regs, int fromipi) "can't continue\n"); remove_bpts(); disable_surveillance(); - /* for breakpoint or single step, print the current instr. */ - if (bp || TRAP(regs) == 0xd00) - ppc_inst_dump(regs->nip, 1, 0); - printf("enter ? for help\n"); + if (!locked_down) { + /* for breakpoint or single step, print current insn */ + if (bp || TRAP(regs) == 0xd00) + ppc_inst_dump(regs->nip, 1, 0); + printf("enter ? for help\n"); + } } - cmd = cmds(regs); + if (!locked_down) + cmd = cmds(regs); insert_bpts(); in_xmon = 0; @@ -677,7 +710,10 @@ static int xmon_core(struct pt_regs *regs, int fromipi) } } #endif - insert_cpu_bpts(); + if (locked_down) + clear_all_bpt(); + else + insert_cpu_bpts(); touch_nmi_watchdog(); local_irq_restore(flags); @@ -3675,6 +3711,11 @@ static void xmon_init(int enable) #ifdef CONFIG_MAGIC_SYSRQ static void sysrq_handle_xmon(int key) { + if (xmon_is_locked_down()) { + clear_all_bpt(); + xmon_init(0); + return; + } /* ensure xmon is enabled */ xmon_init(1); debugger(get_irq_regs()); @@ -3696,12 +3737,39 @@ static int __init setup_xmon_sysrq(void) device_initcall(setup_xmon_sysrq); #endif /* CONFIG_MAGIC_SYSRQ */ +static void clear_all_bpt(void) +{ + int i; + + /* clear/unpatch all breakpoints */ + remove_bpts(); + remove_cpu_bpts(); + + /* Disable all breakpoints */ + for (i = 0; i < NBPTS; ++i) + bpts[i].enabled = 0; + + /* Clear any data or iabr breakpoints */ + if (iabr || dabr.enabled) { + iabr = NULL; + dabr.enabled = 0; + } +} + #ifdef CONFIG_DEBUG_FS static int xmon_dbgfs_set(void *data, u64 val) { xmon_on = !!val; xmon_init(xmon_on); + /* make sure all breakpoints removed when disabling */ + if (!xmon_on) { + clear_all_bpt(); + get_output_lock(); + printf("xmon: All breakpoints cleared\n"); + release_output_lock(); + } + return 0; } @@ -3727,7 +3795,11 @@ static int xmon_early __initdata; static int __init early_parse_xmon(char *p) { - if (!p || strncmp(p, "early", 5) == 0) { + if (xmon_is_locked_down()) { + xmon_init(0); + xmon_early = 0; + xmon_on = 0; + } else if (!p || strncmp(p, "early", 5) == 0) { /* just "xmon" is equivalent to "xmon=early" */ xmon_init(1); xmon_early = 1;