From: David Gow <davidgow@google.com>
To: Kees Cook <kees@kernel.org>,
Linus Torvalds <torvalds@linux-foundation.org>,
Borislav Petkov <bp@alien8.de>
Cc: David Gow <davidgow@google.com>,
Thomas Gleixner <tglx@linutronix.de>,
Ingo Molnar <mingo@redhat.com>,
Dave Hansen <dave.hansen@linux.intel.com>,
Peter Zijlstra <peterz@infradead.org>,
Andrew Morton <akpm@linux-foundation.org>,
"H . Peter Anvin" <hpa@zytor.com>,
x86@kernel.org, kunit-dev@googlegroups.com,
linux-kernel@vger.kernel.org
Subject: [PATCH] x86/uaccess: Zero the 8-byte get_range case on failure
Date: Wed, 31 Jul 2024 15:30:29 +0800 [thread overview]
Message-ID: <20240731073031.4045579-1-davidgow@google.com> (raw)
While zeroing the upper 32 bits of an 8-byte getuser on 32-bit x86 was
fixed by commit 8c860ed825cb ("x86/uaccess: Fix missed zeroing of ia32 u64 get_user() range checking")
it was broken again in commit 8a2462df1547 ("x86/uaccess: Improve the 8-byte getuser() case").
This is because the register which holds the upper 32 bits (%ecx) is
being cleared _after_ the check_range, so if the range check fails, %ecx
is never cleared.
This can be reproduced with:
./tools/testing/kunit/kunit.py run --arch i386 usercopy
Instead, clear %ecx _before_ check_range in the 8-byte case. This
reintroduces a bit of the ugliness we were trying to avoid by adding
another #ifndef CONFIG_X86_64, but at least keeps check_range from
needing a separate bad_get_user_8 jump.
Fixes: 8a2462df1547 ("x86/uaccess: Improve the 8-byte getuser() case")
Signed-off-by: David Gow <davidgow@google.com>
---
There are a few other ways we could fix this, but all of them seem to
increase the ugliness a bit. This seemed the best compromise, but if
you'd prefer to have the size=8 special case live in check_range, that's
fine, too.
See also:
https://lore.kernel.org/lkml/CAHk-=whYb2L_atsRk9pBiFiVLGe5wNZLHhRinA69yu6FiKvDsw@mail.gmail.com/
Cheers,
-- David
---
arch/x86/lib/getuser.S | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/x86/lib/getuser.S b/arch/x86/lib/getuser.S
index a314622aa093..d066aecf8aeb 100644
--- a/arch/x86/lib/getuser.S
+++ b/arch/x86/lib/getuser.S
@@ -88,12 +88,14 @@ SYM_FUNC_END(__get_user_4)
EXPORT_SYMBOL(__get_user_4)
SYM_FUNC_START(__get_user_8)
+#ifndef CONFIG_X86_64
+ xor %ecx,%ecx
+#endif
check_range size=8
ASM_STAC
#ifdef CONFIG_X86_64
UACCESS movq (%_ASM_AX),%rdx
#else
- xor %ecx,%ecx
UACCESS movl (%_ASM_AX),%edx
UACCESS movl 4(%_ASM_AX),%ecx
#endif
--
2.46.0.rc1.232.g9752f9e123-goog
next reply other threads:[~2024-07-31 7:31 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-07-31 7:30 David Gow [this message]
2024-07-31 16:24 ` [PATCH] x86/uaccess: Zero the 8-byte get_range case on failure Linus Torvalds
2024-07-31 16:38 ` Linus Torvalds
2024-08-01 6:34 ` David Gow
2024-08-01 16:24 ` Linus Torvalds
2024-08-01 19:08 ` Thomas Gleixner
2024-08-01 19:28 ` [tip: x86/urgent] x86/uaccess: Zero the 8-byte get_range case on failure on 32-bit tip-bot2 for David Gow
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240731073031.4045579-1-davidgow@google.com \
--to=davidgow@google.com \
--cc=akpm@linux-foundation.org \
--cc=bp@alien8.de \
--cc=dave.hansen@linux.intel.com \
--cc=hpa@zytor.com \
--cc=kees@kernel.org \
--cc=kunit-dev@googlegroups.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=tglx@linutronix.de \
--cc=torvalds@linux-foundation.org \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox