|
Takashi Iwai |
ca3eb6 |
From 5e722b217ad3cf41f5504db80a68062df82b5242 Mon Sep 17 00:00:00 2001
|
|
Takashi Iwai |
ca3eb6 |
From: Ondrej Mosnacek <omosnace@redhat.com>
|
|
Takashi Iwai |
ca3eb6 |
Date: Fri, 7 May 2021 13:57:19 +0200
|
|
Takashi Iwai |
ca3eb6 |
Subject: [PATCH] serial: core: fix suspicious security_locked_down() call
|
|
Takashi Iwai |
ca3eb6 |
Git-commit: 5e722b217ad3cf41f5504db80a68062df82b5242
|
|
Takashi Iwai |
ca3eb6 |
Patch-mainline: v5.13-rc4
|
|
Takashi Iwai |
ca3eb6 |
References: git-fixes
|
|
Takashi Iwai |
ca3eb6 |
|
|
Takashi Iwai |
ca3eb6 |
The commit that added this check did so in a very strange way - first
|
|
Takashi Iwai |
ca3eb6 |
security_locked_down() is called, its value stored into retval, and if
|
|
Takashi Iwai |
ca3eb6 |
it's nonzero, then an additional check is made for (change_irq ||
|
|
Takashi Iwai |
ca3eb6 |
change_port), and if this is true, the function returns. However, if
|
|
Takashi Iwai |
ca3eb6 |
the goto exit branch is not taken, the code keeps the retval value and
|
|
Takashi Iwai |
ca3eb6 |
continues executing the function. Then, depending on whether
|
|
Takashi Iwai |
ca3eb6 |
uport->ops->verify_port is set, the retval value may or may not be reset
|
|
Takashi Iwai |
ca3eb6 |
to zero and eventually the error value from security_locked_down() may
|
|
Takashi Iwai |
ca3eb6 |
abort the function a few lines below.
|
|
Takashi Iwai |
ca3eb6 |
|
|
Takashi Iwai |
ca3eb6 |
I will go out on a limb and assume that this isn't the intended behavior
|
|
Takashi Iwai |
ca3eb6 |
and that an error value from security_locked_down() was supposed to
|
|
Takashi Iwai |
ca3eb6 |
abort the function only in case (change_irq || change_port) is true.
|
|
Takashi Iwai |
ca3eb6 |
|
|
Takashi Iwai |
ca3eb6 |
Note that security_locked_down() should be called last in any series of
|
|
Takashi Iwai |
ca3eb6 |
checks, since the SELinux implementation of this hook will do a check
|
|
Takashi Iwai |
ca3eb6 |
against the policy and generate an audit record in case of denial. If
|
|
Takashi Iwai |
ca3eb6 |
the operation was to carry on after calling security_locked_down(), then
|
|
Takashi Iwai |
ca3eb6 |
the SELinux denial record would be bogus.
|
|
Takashi Iwai |
ca3eb6 |
|
|
Takashi Iwai |
ca3eb6 |
See commit 59438b46471a ("security,lockdown,selinux: implement SELinux
|
|
Takashi Iwai |
ca3eb6 |
lockdown") for how SELinux implements this hook.
|
|
Takashi Iwai |
ca3eb6 |
|
|
Takashi Iwai |
ca3eb6 |
Fixes: 794edf30ee6c ("lockdown: Lock down TIOCSSERIAL")
|
|
Takashi Iwai |
ca3eb6 |
Acked-by: Kees Cook <keescook@chromium.org>
|
|
Takashi Iwai |
ca3eb6 |
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
|
|
Takashi Iwai |
ca3eb6 |
Cc: stable <stable@vger.kernel.org>
|
|
Takashi Iwai |
ca3eb6 |
Link: https://lore.kernel.org/r/20210507115719.140799-1-omosnace@redhat.com
|
|
Takashi Iwai |
ca3eb6 |
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
Takashi Iwai |
ca3eb6 |
Acked-by: Takashi Iwai <tiwai@suse.de>
|
|
Takashi Iwai |
ca3eb6 |
|
|
Takashi Iwai |
ca3eb6 |
---
|
|
Takashi Iwai |
ca3eb6 |
drivers/tty/serial/serial_core.c | 8 +++++---
|
|
Takashi Iwai |
ca3eb6 |
1 file changed, 5 insertions(+), 3 deletions(-)
|
|
Takashi Iwai |
ca3eb6 |
|
|
Takashi Iwai |
ca3eb6 |
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
|
|
Takashi Iwai |
ca3eb6 |
index 87f7127b57e6..18ff85a83f80 100644
|
|
Takashi Iwai |
ca3eb6 |
--- a/drivers/tty/serial/serial_core.c
|
|
Takashi Iwai |
ca3eb6 |
+++ b/drivers/tty/serial/serial_core.c
|
|
Takashi Iwai |
ca3eb6 |
@@ -863,9 +863,11 @@ static int uart_set_info(struct tty_struct *tty, struct tty_port *port,
|
|
Takashi Iwai |
ca3eb6 |
goto check_and_exit;
|
|
Takashi Iwai |
ca3eb6 |
}
|
|
Takashi Iwai |
ca3eb6 |
|
|
Takashi Iwai |
ca3eb6 |
- retval = security_locked_down(LOCKDOWN_TIOCSSERIAL);
|
|
Takashi Iwai |
ca3eb6 |
- if (retval && (change_irq || change_port))
|
|
Takashi Iwai |
ca3eb6 |
- goto exit;
|
|
Takashi Iwai |
ca3eb6 |
+ if (change_irq || change_port) {
|
|
Takashi Iwai |
ca3eb6 |
+ retval = security_locked_down(LOCKDOWN_TIOCSSERIAL);
|
|
Takashi Iwai |
ca3eb6 |
+ if (retval)
|
|
Takashi Iwai |
ca3eb6 |
+ goto exit;
|
|
Takashi Iwai |
ca3eb6 |
+ }
|
|
Takashi Iwai |
ca3eb6 |
|
|
Takashi Iwai |
ca3eb6 |
/*
|
|
Takashi Iwai |
ca3eb6 |
* Ask the low level driver to verify the settings.
|
|
Takashi Iwai |
ca3eb6 |
--
|
|
Takashi Iwai |
ca3eb6 |
2.26.2
|
|
Takashi Iwai |
ca3eb6 |
|