All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] x86/fpu/signal: initialize sw_bytes in save_xstate_epilog()
@ 2021-11-26 12:47 Alexander Potapenko
  2021-11-30 21:08 ` Dave Hansen
  2021-11-30 23:22 ` [tip: x86/urgent] x86/fpu/signal: Initialize " tip-bot2 for Marco Elver
  0 siblings, 2 replies; 5+ messages in thread
From: Alexander Potapenko @ 2021-11-26 12:47 UTC (permalink / raw)
  To: tglx, chang.seok.bae, bp
  Cc: dvyukov, elver, x86, linux-kernel, Alexander Potapenko

save_sw_bytes() did not fully initialize sw_bytes, which caused KMSAN
to report an infoleak (see below).
Initialize sw_bytes explicitly to avoid this.

KMSAN report follows:

=====================================================
BUG: KMSAN: kernel-infoleak in instrument_copy_to_user ./include/linux/instrumented.h:121
BUG: KMSAN: kernel-infoleak in __copy_to_user ./include/linux/uaccess.h:154
BUG: KMSAN: kernel-infoleak in save_xstate_epilog+0x2df/0x510 arch/x86/kernel/fpu/signal.c:127
 instrument_copy_to_user ./include/linux/instrumented.h:121
 __copy_to_user ./include/linux/uaccess.h:154
 save_xstate_epilog+0x2df/0x510 arch/x86/kernel/fpu/signal.c:127
 copy_fpstate_to_sigframe+0x861/0xb60 arch/x86/kernel/fpu/signal.c:245
 get_sigframe+0x656/0x7e0 arch/x86/kernel/signal.c:296
 __setup_rt_frame+0x14d/0x2a60 arch/x86/kernel/signal.c:471
 setup_rt_frame arch/x86/kernel/signal.c:781
 handle_signal arch/x86/kernel/signal.c:825
 arch_do_signal_or_restart+0x417/0xdd0 arch/x86/kernel/signal.c:870
 handle_signal_work kernel/entry/common.c:149
 exit_to_user_mode_loop+0x1f6/0x490 kernel/entry/common.c:173
 exit_to_user_mode_prepare kernel/entry/common.c:208
 __syscall_exit_to_user_mode_work kernel/entry/common.c:290
 syscall_exit_to_user_mode+0x7e/0xc0 kernel/entry/common.c:302
 do_syscall_64+0x60/0xd0 arch/x86/entry/common.c:88
 entry_SYSCALL_64_after_hwframe+0x44/0xae ??:?

Local variable sw_bytes created at:
 save_xstate_epilog+0x80/0x510 arch/x86/kernel/fpu/signal.c:121
 copy_fpstate_to_sigframe+0x861/0xb60 arch/x86/kernel/fpu/signal.c:245

Bytes 20-47 of 48 are uninitialized
Memory access of size 48 starts at ffff8880801d3a18
Data copied to user address 00007ffd90e2ef50
=====================================================

Reported-by: Alexander Potapenko <glider@google.com>
Signed-off-by: Marco Elver <elver@google.com>
Signed-off-by: Alexander Potapenko <glider@google.com>
Tested-by: Alexander Potapenko <glider@google.com>
Fixes: 53599b4d54b9b8dd ("x86/fpu/signal: Prepare for variable sigframe length")
Link: https://lore.kernel.org/all/CAG_fn=V9T6OKPonSjsi9PmWB0hMHFC=yawozdft8i1-MSxrv=w@mail.gmail.com/
---
 arch/x86/kernel/fpu/signal.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c
index d5958278eba6d..91d4b6de58abe 100644
--- a/arch/x86/kernel/fpu/signal.c
+++ b/arch/x86/kernel/fpu/signal.c
@@ -118,7 +118,7 @@ static inline bool save_xstate_epilog(void __user *buf, int ia32_frame,
 				      struct fpstate *fpstate)
 {
 	struct xregs_state __user *x = buf;
-	struct _fpx_sw_bytes sw_bytes;
+	struct _fpx_sw_bytes sw_bytes = {};
 	u32 xfeatures;
 	int err;
 
-- 
2.34.0.rc2.393.gf8c9666880-goog


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] x86/fpu/signal: initialize sw_bytes in save_xstate_epilog()
  2021-11-26 12:47 [PATCH] x86/fpu/signal: initialize sw_bytes in save_xstate_epilog() Alexander Potapenko
@ 2021-11-30 21:08 ` Dave Hansen
  2021-11-30 21:38   ` Alexander Potapenko
  2021-11-30 23:22 ` [tip: x86/urgent] x86/fpu/signal: Initialize " tip-bot2 for Marco Elver
  1 sibling, 1 reply; 5+ messages in thread
From: Dave Hansen @ 2021-11-30 21:08 UTC (permalink / raw)
  To: Alexander Potapenko, tglx, chang.seok.bae, bp
  Cc: dvyukov, elver, x86, linux-kernel

On 11/26/21 4:47 AM, Alexander Potapenko wrote:
> save_sw_bytes() did not fully initialize sw_bytes, which caused KMSAN
> to report an infoleak (see below).
> Initialize sw_bytes explicitly to avoid this.
...
> Reported-by: Alexander Potapenko <glider@google.com>
> Signed-off-by: Marco Elver <elver@google.com>
> Signed-off-by: Alexander Potapenko <glider@google.com>
> Tested-by: Alexander Potapenko <glider@google.com>
> Fixes: 53599b4d54b9b8dd ("x86/fpu/signal: Prepare for variable sigframe length")
> Link: https://lore.kernel.org/all/CAG_fn=V9T6OKPonSjsi9PmWB0hMHFC=yawozdft8i1-MSxrv=w@mail.gmail.com/

Hi Alexander,

Marco's SoB entry is before yours.  Was this authored by you or Marco?
If it was Marco, it's customary to add a:

	From: Marco Elver <elver@google.com>

at the top of the changelog to make sure git gets the author right.  I'm
happy to fix it up this time, I just need to know who wrote it.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] x86/fpu/signal: initialize sw_bytes in save_xstate_epilog()
  2021-11-30 21:08 ` Dave Hansen
@ 2021-11-30 21:38   ` Alexander Potapenko
  2021-11-30 23:28     ` Marco Elver
  0 siblings, 1 reply; 5+ messages in thread
From: Alexander Potapenko @ 2021-11-30 21:38 UTC (permalink / raw)
  To: Dave Hansen; +Cc: tglx, chang.seok.bae, bp, dvyukov, elver, x86, linux-kernel

On Tue, Nov 30, 2021 at 10:09 PM Dave Hansen <dave.hansen@intel.com> wrote:
>
> On 11/26/21 4:47 AM, Alexander Potapenko wrote:
> > save_sw_bytes() did not fully initialize sw_bytes, which caused KMSAN
> > to report an infoleak (see below).
> > Initialize sw_bytes explicitly to avoid this.
> ...
> > Reported-by: Alexander Potapenko <glider@google.com>
> > Signed-off-by: Marco Elver <elver@google.com>
> > Signed-off-by: Alexander Potapenko <glider@google.com>
> > Tested-by: Alexander Potapenko <glider@google.com>
> > Fixes: 53599b4d54b9b8dd ("x86/fpu/signal: Prepare for variable sigframe length")
> > Link: https://lore.kernel.org/all/CAG_fn=V9T6OKPonSjsi9PmWB0hMHFC=yawozdft8i1-MSxrv=w@mail.gmail.com/
>
> Hi Alexander,
>
> Marco's SoB entry is before yours.  Was this authored by you or Marco?
> If it was Marco, it's customary to add a:
>
>         From: Marco Elver <elver@google.com>
>
> at the top of the changelog to make sure git gets the author right.  I'm
> happy to fix it up this time, I just need to know who wrote it.

Hi Dave,

Yes, it was authored by Marco. Thanks in advance for fixing this, I'll
keep that in mind next time :)


-- 
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-Straße, 33
80636 München

Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [tip: x86/urgent] x86/fpu/signal: Initialize sw_bytes in save_xstate_epilog()
  2021-11-26 12:47 [PATCH] x86/fpu/signal: initialize sw_bytes in save_xstate_epilog() Alexander Potapenko
  2021-11-30 21:08 ` Dave Hansen
@ 2021-11-30 23:22 ` tip-bot2 for Marco Elver
  1 sibling, 0 replies; 5+ messages in thread
From: tip-bot2 for Marco Elver @ 2021-11-30 23:22 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Alexander Potapenko, Marco Elver, Dave Hansen, x86, linux-kernel

The following commit has been merged into the x86/urgent branch of tip:

Commit-ID:     52d0b8b18776f184c53632c5e0068201491cdb61
Gitweb:        https://git.kernel.org/tip/52d0b8b18776f184c53632c5e0068201491cdb61
Author:        Marco Elver <elver@google.com>
AuthorDate:    Fri, 26 Nov 2021 13:47:46 +01:00
Committer:     Dave Hansen <dave.hansen@linux.intel.com>
CommitterDate: Tue, 30 Nov 2021 15:13:47 -08:00

x86/fpu/signal: Initialize sw_bytes in save_xstate_epilog()

save_sw_bytes() did not fully initialize sw_bytes, which caused KMSAN
to report an infoleak (see below).
Initialize sw_bytes explicitly to avoid this.

KMSAN report follows:

=====================================================
BUG: KMSAN: kernel-infoleak in instrument_copy_to_user ./include/linux/instrumented.h:121
BUG: KMSAN: kernel-infoleak in __copy_to_user ./include/linux/uaccess.h:154
BUG: KMSAN: kernel-infoleak in save_xstate_epilog+0x2df/0x510 arch/x86/kernel/fpu/signal.c:127
 instrument_copy_to_user ./include/linux/instrumented.h:121
 __copy_to_user ./include/linux/uaccess.h:154
 save_xstate_epilog+0x2df/0x510 arch/x86/kernel/fpu/signal.c:127
 copy_fpstate_to_sigframe+0x861/0xb60 arch/x86/kernel/fpu/signal.c:245
 get_sigframe+0x656/0x7e0 arch/x86/kernel/signal.c:296
 __setup_rt_frame+0x14d/0x2a60 arch/x86/kernel/signal.c:471
 setup_rt_frame arch/x86/kernel/signal.c:781
 handle_signal arch/x86/kernel/signal.c:825
 arch_do_signal_or_restart+0x417/0xdd0 arch/x86/kernel/signal.c:870
 handle_signal_work kernel/entry/common.c:149
 exit_to_user_mode_loop+0x1f6/0x490 kernel/entry/common.c:173
 exit_to_user_mode_prepare kernel/entry/common.c:208
 __syscall_exit_to_user_mode_work kernel/entry/common.c:290
 syscall_exit_to_user_mode+0x7e/0xc0 kernel/entry/common.c:302
 do_syscall_64+0x60/0xd0 arch/x86/entry/common.c:88
 entry_SYSCALL_64_after_hwframe+0x44/0xae ??:?

Local variable sw_bytes created at:
 save_xstate_epilog+0x80/0x510 arch/x86/kernel/fpu/signal.c:121
 copy_fpstate_to_sigframe+0x861/0xb60 arch/x86/kernel/fpu/signal.c:245

Bytes 20-47 of 48 are uninitialized
Memory access of size 48 starts at ffff8880801d3a18
Data copied to user address 00007ffd90e2ef50
=====================================================

Link: https://lore.kernel.org/all/CAG_fn=V9T6OKPonSjsi9PmWB0hMHFC=yawozdft8i1-MSxrv=w@mail.gmail.com/
Fixes: 53599b4d54b9b8dd ("x86/fpu/signal: Prepare for variable sigframe length")
Reported-by: Alexander Potapenko <glider@google.com>
Signed-off-by: Marco Elver <elver@google.com>
Signed-off-by: Alexander Potapenko <glider@google.com>
Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
Tested-by: Alexander Potapenko <glider@google.com>
Link: https://lkml.kernel.org/r/20211126124746.761278-1-glider@google.com
---
 arch/x86/kernel/fpu/signal.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c
index d595827..91d4b6d 100644
--- a/arch/x86/kernel/fpu/signal.c
+++ b/arch/x86/kernel/fpu/signal.c
@@ -118,7 +118,7 @@ static inline bool save_xstate_epilog(void __user *buf, int ia32_frame,
 				      struct fpstate *fpstate)
 {
 	struct xregs_state __user *x = buf;
-	struct _fpx_sw_bytes sw_bytes;
+	struct _fpx_sw_bytes sw_bytes = {};
 	u32 xfeatures;
 	int err;
 

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] x86/fpu/signal: initialize sw_bytes in save_xstate_epilog()
  2021-11-30 21:38   ` Alexander Potapenko
@ 2021-11-30 23:28     ` Marco Elver
  0 siblings, 0 replies; 5+ messages in thread
From: Marco Elver @ 2021-11-30 23:28 UTC (permalink / raw)
  To: Alexander Potapenko
  Cc: Dave Hansen, tglx, chang.seok.bae, bp, dvyukov, x86, linux-kernel

On Tue, 30 Nov 2021 at 22:39, Alexander Potapenko <glider@google.com> wrote:
>
> On Tue, Nov 30, 2021 at 10:09 PM Dave Hansen <dave.hansen@intel.com> wrote:
> >
> > On 11/26/21 4:47 AM, Alexander Potapenko wrote:
> > > save_sw_bytes() did not fully initialize sw_bytes, which caused KMSAN
> > > to report an infoleak (see below).
> > > Initialize sw_bytes explicitly to avoid this.
> > ...
> > > Reported-by: Alexander Potapenko <glider@google.com>
> > > Signed-off-by: Marco Elver <elver@google.com>
> > > Signed-off-by: Alexander Potapenko <glider@google.com>
> > > Tested-by: Alexander Potapenko <glider@google.com>
> > > Fixes: 53599b4d54b9b8dd ("x86/fpu/signal: Prepare for variable sigframe length")
> > > Link: https://lore.kernel.org/all/CAG_fn=V9T6OKPonSjsi9PmWB0hMHFC=yawozdft8i1-MSxrv=w@mail.gmail.com/
> >
> > Hi Alexander,
> >
> > Marco's SoB entry is before yours.  Was this authored by you or Marco?
> > If it was Marco, it's customary to add a:
> >
> >         From: Marco Elver <elver@google.com>
> >
> > at the top of the changelog to make sure git gets the author right.  I'm
> > happy to fix it up this time, I just need to know who wrote it.
>
> Hi Dave,
>
> Yes, it was authored by Marco. Thanks in advance for fixing this, I'll
> keep that in mind next time :)

I produced a diff, but Alex tested and wrote the commit message.

I'm happy to have Alex be the primary author here, he spent more time
on this. :-)
If you want it to be more precise, you could add a:

Co-developed-by: Marco Elver <elver@google.com>

Thanks,
-- Marco

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-11-30 23:29 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-26 12:47 [PATCH] x86/fpu/signal: initialize sw_bytes in save_xstate_epilog() Alexander Potapenko
2021-11-30 21:08 ` Dave Hansen
2021-11-30 21:38   ` Alexander Potapenko
2021-11-30 23:28     ` Marco Elver
2021-11-30 23:22 ` [tip: x86/urgent] x86/fpu/signal: Initialize " tip-bot2 for Marco Elver

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.