[2/2,SRU,E] UBUNTU: SAUCE: (efi-lockdown) Really don't allow lifting lockdown from userspace
diff mbox series

Message ID 20191105203505.28634-3-seth.forshee@canonical.com
State New
Headers show
Series
  • Don't allow lifting of lockdown via /proc/sysrq-trigger
Related show

Commit Message

Seth Forshee Nov. 5, 2019, 8:35 p.m. UTC
BugLink: https://bugs.launchpad.net/bugs/1851380

"UBUNTU: SAUCE: (efi-lockdown) Add a SysRq option to lift kernel
lockdown" adds a sysrq key to lift kernel lockdown, which is
meant to only allow a physically present user to lift lockdown
using a keyboard. However, the code has a bug which also allows
root to lift lockdown through /proc/sysrq-trigger. Fix this bug
to make this work as intended.

Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
 drivers/tty/sysrq.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Comments

Tyler Hicks Nov. 5, 2019, 8:42 p.m. UTC | #1
On 2019-11-05 14:35:05, Seth Forshee wrote:
> BugLink: https://bugs.launchpad.net/bugs/1851380
> 
> "UBUNTU: SAUCE: (efi-lockdown) Add a SysRq option to lift kernel
> lockdown" adds a sysrq key to lift kernel lockdown, which is
> meant to only allow a physically present user to lift lockdown
> using a keyboard. However, the code has a bug which also allows
> root to lift lockdown through /proc/sysrq-trigger. Fix this bug
> to make this work as intended.
> 
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>

Acked-by: Tyler Hicks <tyhicks@canonical.com>

Tyler

> ---
>  drivers/tty/sysrq.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
> index 7cc95a8bdf8d..99082faafc44 100644
> --- a/drivers/tty/sysrq.c
> +++ b/drivers/tty/sysrq.c
> @@ -549,13 +549,13 @@ void __handle_sysrq(int key, unsigned int from)
>          if (op_p) {
>  		/* Ban synthetic events from some sysrq functionality */
>  		if ((from == SYSRQ_FROM_PROC || from == SYSRQ_FROM_SYNTHETIC) &&
> -		    op_p->enable_mask & SYSRQ_DISABLE_USERSPACE)
> +		    op_p->enable_mask & SYSRQ_DISABLE_USERSPACE) {
>  			printk("This sysrq operation is disabled from userspace.\n");
> -		/*
> -		 * Should we check for enabled operations (/proc/sysrq-trigger
> -		 * should not) and is the invoked operation enabled?
> -		 */
> -		if (from == SYSRQ_FROM_KERNEL || sysrq_on_mask(op_p->enable_mask)) {
> +		} else if (from == SYSRQ_FROM_KERNEL || sysrq_on_mask(op_p->enable_mask)) {
> +			/*
> +			 * Should we check for enabled operations (/proc/sysrq-trigger
> +			 * should not) and is the invoked operation enabled?
> +			 */
>  			pr_info("%s\n", op_p->action_msg);
>  			console_loglevel = orig_log_level;
>  			op_p->handler(key);
> -- 
> 2.20.1
> 
> 
> -- 
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

Patch
diff mbox series

diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
index 7cc95a8bdf8d..99082faafc44 100644
--- a/drivers/tty/sysrq.c
+++ b/drivers/tty/sysrq.c
@@ -549,13 +549,13 @@  void __handle_sysrq(int key, unsigned int from)
         if (op_p) {
 		/* Ban synthetic events from some sysrq functionality */
 		if ((from == SYSRQ_FROM_PROC || from == SYSRQ_FROM_SYNTHETIC) &&
-		    op_p->enable_mask & SYSRQ_DISABLE_USERSPACE)
+		    op_p->enable_mask & SYSRQ_DISABLE_USERSPACE) {
 			printk("This sysrq operation is disabled from userspace.\n");
-		/*
-		 * Should we check for enabled operations (/proc/sysrq-trigger
-		 * should not) and is the invoked operation enabled?
-		 */
-		if (from == SYSRQ_FROM_KERNEL || sysrq_on_mask(op_p->enable_mask)) {
+		} else if (from == SYSRQ_FROM_KERNEL || sysrq_on_mask(op_p->enable_mask)) {
+			/*
+			 * Should we check for enabled operations (/proc/sysrq-trigger
+			 * should not) and is the invoked operation enabled?
+			 */
 			pr_info("%s\n", op_p->action_msg);
 			console_loglevel = orig_log_level;
 			op_p->handler(key);