On a 32-bit fast syscall that fails to read its arguments from user memory, the kernel currently does syscall exit work but not syscall exit work. This would confuse audit and ptrace.
This is a minimal fix intended for ease of backporting. A more complete cleanup is coming.
Cc: stable@vger.kernel.org Fixes: 0b085e68f407 ("x86/entry: Consolidate 32/64 bit syscall entry") Signed-off-by: Andy Lutomirski luto@kernel.org --- arch/x86/entry/common.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c index 0904f5676e4d..cf4dcf346ca8 100644 --- a/arch/x86/entry/common.c +++ b/arch/x86/entry/common.c @@ -128,7 +128,8 @@ static noinstr bool __do_fast_syscall_32(struct pt_regs *regs) regs->ax = -EFAULT;
instrumentation_end(); - syscall_exit_to_user_mode(regs); + local_irq_disable(); + exit_to_user_mode(); return false; }
On Mon, Feb 22 2021 at 21:50, Andy Lutomirski wrote:
On a 32-bit fast syscall that fails to read its arguments from user memory, the kernel currently does syscall exit work but not syscall exit work.
and this sentence does not make any sense.
This would confuse audit and ptrace.
Would? It does confuse it, right?
This is a minimal fix intended for ease of backporting. A more complete cleanup is coming.
Cc: stable@vger.kernel.org Fixes: 0b085e68f407 ("x86/entry: Consolidate 32/64 bit syscall entry") Signed-off-by: Andy Lutomirski luto@kernel.org
arch/x86/entry/common.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c index 0904f5676e4d..cf4dcf346ca8 100644 --- a/arch/x86/entry/common.c +++ b/arch/x86/entry/common.c @@ -128,7 +128,8 @@ static noinstr bool __do_fast_syscall_32(struct pt_regs *regs) regs->ax = -EFAULT; instrumentation_end();
syscall_exit_to_user_mode(regs);
local_irq_disable();exit_to_user_mode();
While I suggested this with tired brain yesterday night, it's not really correct as it does not go through exit_to_user_mode_prepare() which it really has to.
Thanks,
tglx
On Mon, Feb 22, 2021 at 09:50:28PM -0800, Andy Lutomirski wrote:
On a 32-bit fast syscall that fails to read its arguments from user memory, the kernel currently does syscall exit work but not syscall exit work. This would confuse audit and ptrace.
This is a minimal fix intended for ease of backporting. A more complete cleanup is coming.
Cc: stable@vger.kernel.org Fixes: 0b085e68f407 ("x86/entry: Consolidate 32/64 bit syscall entry") Signed-off-by: Andy Lutomirski luto@kernel.org
arch/x86/entry/common.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c index 0904f5676e4d..cf4dcf346ca8 100644 --- a/arch/x86/entry/common.c +++ b/arch/x86/entry/common.c @@ -128,7 +128,8 @@ static noinstr bool __do_fast_syscall_32(struct pt_regs *regs) regs->ax = -EFAULT; instrumentation_end();
syscall_exit_to_user_mode(regs);
local_irq_disable();exit_to_user_mode();
I'm confused, twice. Once by your Changelog, and second by the actual patch. Shouldn't every return to userspace pass through exit_to_user_mode_prepare() ? We shouldn't ignore NEED_RESCHED or NOTIFY_RESUME, both of which can be set I think, even if the SYSCALL didn't actually do anything.
On Feb 23, 2021, at 3:29 AM, Peter Zijlstra peterz@infradead.org wrote:
On Mon, Feb 22, 2021 at 09:50:28PM -0800, Andy Lutomirski wrote:
On a 32-bit fast syscall that fails to read its arguments from user memory, the kernel currently does syscall exit work but not syscall exit work. This would confuse audit and ptrace.
This is a minimal fix intended for ease of backporting. A more complete cleanup is coming.
Cc: stable@vger.kernel.org Fixes: 0b085e68f407 ("x86/entry: Consolidate 32/64 bit syscall entry") Signed-off-by: Andy Lutomirski luto@kernel.org
arch/x86/entry/common.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c index 0904f5676e4d..cf4dcf346ca8 100644 --- a/arch/x86/entry/common.c +++ b/arch/x86/entry/common.c @@ -128,7 +128,8 @@ static noinstr bool __do_fast_syscall_32(struct pt_regs *regs) regs->ax = -EFAULT;
instrumentation_end();
syscall_exit_to_user_mode(regs);
local_irq_disable();exit_to_user_mode(); return false;I'm confused, twice. Once by your Changelog, and second by the actual patch. Shouldn't every return to userspace pass through exit_to_user_mode_prepare() ? We shouldn't ignore NEED_RESCHED or NOTIFY_RESUME, both of which can be set I think, even if the SYSCALL didn't actually do anything.
Aaaaahhhhhh! There are too many of these functions. I’ll poke around. I’ll also try to figure out why I didn’t catch this in testing.
linux-stable-mirror@lists.linaro.org
-
Andy Lutomirski -
Andy Lutomirski -
Peter Zijlstra -
Thomas Gleixner