diff mbox series

CVE-2023-0459: Spectre-v1 Usercopy Hardening was Re: [cip-dev] New CVE entries this week

Message ID ZEGcHOWAk8S0zEBz@duo.ucw.cz (mailing list archive)
State New
Headers show
Series CVE-2023-0459: Spectre-v1 Usercopy Hardening was Re: [cip-dev] New CVE entries this week | expand

Commit Message

Pavel Machek April 20, 2023, 8:10 p.m. UTC
Hi!

> CVE-2023-0459: Spectre-v1 Usercopy Hardening
> 
> CVSS v3 score is not provided.
> 
> Missing speculation barriers causes a leaking kernel memory.
> The 4.4 kernel _copy_from_user() implementation is different from
> 4.19 and later. So this patch can not be applied.
> It seems as if the 4.4 kernel needs some barrier to prevent
> speculation bug in other ways.

I have this, and it passes basic testing, but no good way to really
test it or asses performance impact.

    Attempt to fix CVE-2023-0459. Inspiration from
    e0fbff18bbcee4f07d46bee172803fad63f6f4dd, but we simply add the check
    to access_ok, as it is used in about gazillion places.

Best regards,
								Pavel

Comments

Masami Ichikawa April 21, 2023, 7:27 a.m. UTC | #1
Hi.

On Fri, Apr 21, 2023 at 5:10 AM Pavel Machek <pavel@denx.de> wrote:
>
> Hi!
>
> > CVE-2023-0459: Spectre-v1 Usercopy Hardening
> >
> > CVSS v3 score is not provided.
> >
> > Missing speculation barriers causes a leaking kernel memory.
> > The 4.4 kernel _copy_from_user() implementation is different from
> > 4.19 and later. So this patch can not be applied.
> > It seems as if the 4.4 kernel needs some barrier to prevent
> > speculation bug in other ways.
>
> I have this, and it passes basic testing,

Nice!

> but no good way to really
> test it or asses performance impact.
>

Umm, we need to write a test driver and check performance by perf or something?
I found the Linux Test Project write a test driver to test
copy_[from|to]_user and [put|get]_user.  We may need the same thing.
https://github.com/linux-test-project/ltp/tree/master/testcases/kernel/device-drivers/uaccess

>     Attempt to fix CVE-2023-0459. Inspiration from
>     e0fbff18bbcee4f07d46bee172803fad63f6f4dd, but we simply add the check
>     to access_ok, as it is used in about gazillion places.
>
> Best regards,
>                                                                 Pavel
>
> diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
> index 829fa6d3e5611..ce45b6dcb6293 100644
> --- a/arch/arm64/include/asm/uaccess.h
> +++ b/arch/arm64/include/asm/uaccess.h
> @@ -101,6 +101,7 @@ static inline void set_fs(mm_segment_t fs)
>                 : "1" (__addr), "Ir" (size),                            \
>                   "r" (current_thread_info()->addr_limit)               \
>                 : "cc");                                                \
> +       barrier_nospec();                                               \
>         flag;                                                           \
>  })
>
> diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
> index ac6932bf1a016..ea1e4ef4a4d8e 100644
> --- a/arch/x86/include/asm/uaccess.h
> +++ b/arch/x86/include/asm/uaccess.h
> @@ -95,8 +95,11 @@ static inline bool __chk_range_not_ok(unsigned long addr, unsigned long size, un
>   */
>  #define access_ok(type, addr, size)                                    \
>  ({                                                                     \
> +       bool ret;                                                       \
>         WARN_ON_IN_IRQ();                                               \
> -       likely(!__range_not_ok(addr, size, user_addr_max()));           \
> +       ret = likely(!__range_not_ok(addr, size, user_addr_max()));     \
> +       barrier_nospec();                                               \
> +       ret;                                                            \
>  })
>
>  /*
> diff --git a/include/linux/nospec.h b/include/linux/nospec.h
> index 0c5ef54fd4162..207ef2a20e485 100644
> --- a/include/linux/nospec.h
> +++ b/include/linux/nospec.h
> @@ -9,6 +9,10 @@
>
>  struct task_struct;
>
> +#ifndef barrier_nospec
> +# define barrier_nospec() do { } while (0)
> +#endif
> +
>  /**
>   * array_index_mask_nospec() - generate a ~0 mask when index < size, 0 otherwise
>   * @index: array element index
>
>
> --
> DENX Software Engineering GmbH,        Managing Director: Erika Unter
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#11264): https://lists.cip-project.org/g/cip-dev/message/11264
> Mute This Topic: https://lists.cip-project.org/mt/98397283/4520416
> Group Owner: cip-dev+owner@lists.cip-project.org
> Unsubscribe: https://lists.cip-project.org/g/cip-dev/leave/8129101/4520416/1465703922/xyzzy [masami.ichikawa@miraclelinux.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>

Regards,
diff mbox series

Patch

diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
index 829fa6d3e5611..ce45b6dcb6293 100644
--- a/arch/arm64/include/asm/uaccess.h
+++ b/arch/arm64/include/asm/uaccess.h
@@ -101,6 +101,7 @@  static inline void set_fs(mm_segment_t fs)
 		: "1" (__addr), "Ir" (size),				\
 		  "r" (current_thread_info()->addr_limit)		\
 		: "cc");						\
+	barrier_nospec();						\
 	flag;								\
 })
 
diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
index ac6932bf1a016..ea1e4ef4a4d8e 100644
--- a/arch/x86/include/asm/uaccess.h
+++ b/arch/x86/include/asm/uaccess.h
@@ -95,8 +95,11 @@  static inline bool __chk_range_not_ok(unsigned long addr, unsigned long size, un
  */
 #define access_ok(type, addr, size)					\
 ({									\
+	bool ret;							\
 	WARN_ON_IN_IRQ();						\
-	likely(!__range_not_ok(addr, size, user_addr_max()));		\
+	ret = likely(!__range_not_ok(addr, size, user_addr_max()));	\
+	barrier_nospec();						\
+	ret;								\
 })
 
 /*
diff --git a/include/linux/nospec.h b/include/linux/nospec.h
index 0c5ef54fd4162..207ef2a20e485 100644
--- a/include/linux/nospec.h
+++ b/include/linux/nospec.h
@@ -9,6 +9,10 @@ 
 
 struct task_struct;
 
+#ifndef barrier_nospec
+# define barrier_nospec() do { } while (0)
+#endif
+
 /**
  * array_index_mask_nospec() - generate a ~0 mask when index < size, 0 otherwise
  * @index: array element index