LinuxLists.cc - [PATCH for 4.19] tracepoint: Fix: out-of-bound tracepoint array iteration

2018-10-12 20:07:25

Subject: [PATCH for 4.19] tracepoint: Fix: out-of-bound tracepoint array iteration

commit 46e0c9be206f ("kernel: tracepoints: add support for relative
references") changes the layout of the __tracepoint_ptrs section on
architectures supporting relative references. However, it does so
without turning struct tracepoint * const into const int * elsewhere in
the tracepoint code, which has the following side-effect:

tracepoint_module_{coming,going} invoke
tp_module_going_check_quiescent() with mod->tracepoints_ptrs
as first argument, and computes the end address of the array
for the second argument with:

mod->tracepoints_ptrs + mod->num_tracepoints

However, because the type of mod->tracepoint_ptrs in module.h
has not been changed from pointer to int, it passes an end
pointer which is twice larger than the array, causing out-of-bound
array accesses.

Fix this by introducing a new typedef: tracepoint_ptr_t, which
is either "const int" on architectures that have PREL32 relocations,
or "struct tracepoint * const" on architectures that does not have
this feature.

Also provide a new tracepoint_ptr_defer() static inline to
encapsulate deferencing this type rather than duplicate code and
ugly idefs within the for_each_tracepoint_range() implementation.

This issue appears in 4.19-rc kernels, and should ideally be fixed
before the end of the rc cycle.

Signed-off-by: Mathieu Desnoyers <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Cc: Michael Ellerman <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Steven Rostedt (VMware) <[email protected]>
Cc: Ard Biesheuvel <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Benjamin Herrenschmidt <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: James Morris <[email protected]>
Cc: James Morris <[email protected]>
Cc: Jessica Yu <[email protected]>
Cc: Josh Poimboeuf <[email protected]>
Cc: Kees Cook <[email protected]>
Cc: Nicolas Pitre <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Petr Mladek <[email protected]>
Cc: Russell King <[email protected]>
Cc: "Serge E. Hallyn" <[email protected]>
Cc: Sergey Senozhatsky <[email protected]>
Cc: Thomas Garnier <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
---
include/linux/module.h | 2 +-
include/linux/tracepoint-defs.h | 6 ++++++
include/linux/tracepoint.h | 36 +++++++++++++++++++++------------
kernel/tracepoint.c | 24 ++++++++--------------
4 files changed, 38 insertions(+), 30 deletions(-)

diff --git a/include/linux/module.h b/include/linux/module.h
index f807f15bebbe..cdab2451d6be 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -430,7 +430,7 @@ struct module {

#ifdef CONFIG_TRACEPOINTS
unsigned int num_tracepoints;
- struct tracepoint * const *tracepoints_ptrs;
+ tracepoint_ptr_t *tracepoints_ptrs;
#endif
#ifdef HAVE_JUMP_LABEL
struct jump_entry *jump_entries;
diff --git a/include/linux/tracepoint-defs.h b/include/linux/tracepoint-defs.h
index 22c5a46e9693..49ba9cde7e4b 100644
--- a/include/linux/tracepoint-defs.h
+++ b/include/linux/tracepoint-defs.h
@@ -35,6 +35,12 @@ struct tracepoint {
struct tracepoint_func __rcu *funcs;
};

+#ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
+typedef const int tracepoint_ptr_t;
+#else
+typedef struct tracepoint * const tracepoint_ptr_t;
+#endif
+
struct bpf_raw_event_map {
struct tracepoint *tp;
void *bpf_func;
diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
index 041f7e56a289..538ba1a58f5b 100644
--- a/include/linux/tracepoint.h
+++ b/include/linux/tracepoint.h
@@ -99,6 +99,29 @@ extern void syscall_unregfunc(void);
#define TRACE_DEFINE_ENUM(x)
#define TRACE_DEFINE_SIZEOF(x)

+#ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
+static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
+{
+ return offset_to_ptr(p);
+}
+
+#define __TRACEPOINT_ENTRY(name) \
+ asm(" .section \"__tracepoints_ptrs\", \"a\" \n" \
+ " .balign 4 \n" \
+ " .long __tracepoint_" #name " - . \n" \
+ " .previous \n")
+#else
+static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
+{
+ return *p;
+}
+
+#define __TRACEPOINT_ENTRY(name) \
+ static tracepoint_ptr_t __tracepoint_ptr_##name __used \
+ __attribute__((section("__tracepoints_ptrs"))) = \
+ &__tracepoint_##name
+#endif
+
#endif /* _LINUX_TRACEPOINT_H */

/*
@@ -253,19 +276,6 @@ extern void syscall_unregfunc(void);
return static_key_false(&__tracepoint_##name.key); \
}

-#ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
-#define __TRACEPOINT_ENTRY(name) \
- asm(" .section \"__tracepoints_ptrs\", \"a\" \n" \
- " .balign 4 \n" \
- " .long __tracepoint_" #name " - . \n" \
- " .previous \n")
-#else
-#define __TRACEPOINT_ENTRY(name) \
- static struct tracepoint * const __tracepoint_ptr_##name __used \
- __attribute__((section("__tracepoints_ptrs"))) = \
- &__tracepoint_##name
-#endif
-
/*
* We have no guarantee that gcc and the linker won't up-align the tracepoint
* structures, so we create an array of pointers that will be used for iteration
diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
index bf2c06ef9afc..a3be42304485 100644
--- a/kernel/tracepoint.c
+++ b/kernel/tracepoint.c
@@ -28,8 +28,8 @@
#include <linux/sched/task.h>
#include <linux/static_key.h>

-extern struct tracepoint * const __start___tracepoints_ptrs[];
-extern struct tracepoint * const __stop___tracepoints_ptrs[];
+extern tracepoint_ptr_t __start___tracepoints_ptrs[];
+extern tracepoint_ptr_t __stop___tracepoints_ptrs[];

DEFINE_SRCU(tracepoint_srcu);
EXPORT_SYMBOL_GPL(tracepoint_srcu);
@@ -371,25 +371,17 @@ int tracepoint_probe_unregister(struct tracepoint *tp, void *probe, void *data)
}
EXPORT_SYMBOL_GPL(tracepoint_probe_unregister);

-static void for_each_tracepoint_range(struct tracepoint * const *begin,
- struct tracepoint * const *end,
+static void for_each_tracepoint_range(
+ tracepoint_ptr_t *begin, tracepoint_ptr_t *end,
void (*fct)(struct tracepoint *tp, void *priv),
void *priv)
{
+ tracepoint_ptr_t *iter;
+
if (!begin)
return;
-
- if (IS_ENABLED(CONFIG_HAVE_ARCH_PREL32_RELOCATIONS)) {
- const int *iter;
-
- for (iter = (const int *)begin; iter < (const int *)end; iter++)
- fct(offset_to_ptr(iter), priv);
- } else {
- struct tracepoint * const *iter;
-
- for (iter = begin; iter < end; iter++)
- fct(*iter, priv);
- }
+ for (iter = begin; iter < end; iter++)
+ fct(tracepoint_ptr_deref(iter), priv);
}

#ifdef CONFIG_MODULES
--
2.17.1

2018-10-12 20:20:43

by Mathieu Desnoyers

[permalink] [raw]

Subject: Re: [PATCH for 4.19] tracepoint: Fix: out-of-bound tracepoint array iteration

----- On Oct 12, 2018, at 4:05 PM, Mathieu Desnoyers [email protected] wrote:

> commit 46e0c9be206f ("kernel: tracepoints: add support for relative
> references") changes the layout of the __tracepoint_ptrs section on
> architectures supporting relative references. However, it does so
> without turning struct tracepoint * const into const int * elsewhere in
> the tracepoint code, which has the following side-effect:

Actually, the above sentence should read:

"However, it does so without turning struct tracepoint * const into const int
elsewhere in the tracepoint code [...]" (I had mistakenly worded it "const int *").

Thanks,

Mathieu

>
> tracepoint_module_{coming,going} invoke
> tp_module_going_check_quiescent() with mod->tracepoints_ptrs
> as first argument, and computes the end address of the array
> for the second argument with:
>
> mod->tracepoints_ptrs + mod->num_tracepoints
>
> However, because the type of mod->tracepoint_ptrs in module.h
> has not been changed from pointer to int, it passes an end
> pointer which is twice larger than the array, causing out-of-bound
> array accesses.
>
> Fix this by introducing a new typedef: tracepoint_ptr_t, which
> is either "const int" on architectures that have PREL32 relocations,
> or "struct tracepoint * const" on architectures that does not have
> this feature.
>
> Also provide a new tracepoint_ptr_defer() static inline to
> encapsulate deferencing this type rather than duplicate code and
> ugly idefs within the for_each_tracepoint_range() implementation.
>
> This issue appears in 4.19-rc kernels, and should ideally be fixed
> before the end of the rc cycle.
>
> Signed-off-by: Mathieu Desnoyers <[email protected]>
> Link: http://lkml.kernel.org/r/[email protected]
> Cc: Michael Ellerman <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Steven Rostedt (VMware) <[email protected]>
> Cc: Ard Biesheuvel <[email protected]>
> Cc: Arnd Bergmann <[email protected]>
> Cc: Benjamin Herrenschmidt <[email protected]>
> Cc: Bjorn Helgaas <[email protected]>
> Cc: Catalin Marinas <[email protected]>
> Cc: James Morris <[email protected]>
> Cc: James Morris <[email protected]>
> Cc: Jessica Yu <[email protected]>
> Cc: Josh Poimboeuf <[email protected]>
> Cc: Kees Cook <[email protected]>
> Cc: Nicolas Pitre <[email protected]>
> Cc: Paul Mackerras <[email protected]>
> Cc: Petr Mladek <[email protected]>
> Cc: Russell King <[email protected]>
> Cc: "Serge E. Hallyn" <[email protected]>
> Cc: Sergey Senozhatsky <[email protected]>
> Cc: Thomas Garnier <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Will Deacon <[email protected]>
> Cc: Andrew Morton <[email protected]>
> Cc: Linus Torvalds <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> ---
> include/linux/module.h | 2 +-
> include/linux/tracepoint-defs.h | 6 ++++++
> include/linux/tracepoint.h | 36 +++++++++++++++++++++------------
> kernel/tracepoint.c | 24 ++++++++--------------
> 4 files changed, 38 insertions(+), 30 deletions(-)
>
> diff --git a/include/linux/module.h b/include/linux/module.h
> index f807f15bebbe..cdab2451d6be 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -430,7 +430,7 @@ struct module {
>
> #ifdef CONFIG_TRACEPOINTS
> unsigned int num_tracepoints;
> - struct tracepoint * const *tracepoints_ptrs;
> + tracepoint_ptr_t *tracepoints_ptrs;
> #endif
> #ifdef HAVE_JUMP_LABEL
> struct jump_entry *jump_entries;
> diff --git a/include/linux/tracepoint-defs.h b/include/linux/tracepoint-defs.h
> index 22c5a46e9693..49ba9cde7e4b 100644
> --- a/include/linux/tracepoint-defs.h
> +++ b/include/linux/tracepoint-defs.h
> @@ -35,6 +35,12 @@ struct tracepoint {
> struct tracepoint_func __rcu *funcs;
> };
>
> +#ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
> +typedef const int tracepoint_ptr_t;
> +#else
> +typedef struct tracepoint * const tracepoint_ptr_t;
> +#endif
> +
> struct bpf_raw_event_map {
> struct tracepoint *tp;
> void *bpf_func;
> diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
> index 041f7e56a289..538ba1a58f5b 100644
> --- a/include/linux/tracepoint.h
> +++ b/include/linux/tracepoint.h
> @@ -99,6 +99,29 @@ extern void syscall_unregfunc(void);
> #define TRACE_DEFINE_ENUM(x)
> #define TRACE_DEFINE_SIZEOF(x)
>
> +#ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
> +static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
> +{
> + return offset_to_ptr(p);
> +}
> +
> +#define __TRACEPOINT_ENTRY(name) \
> + asm(" .section \"__tracepoints_ptrs\", \"a\" \n" \
> + " .balign 4 \n" \
> + " .long __tracepoint_" #name " - . \n" \
> + " .previous \n")
> +#else
> +static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
> +{
> + return *p;
> +}
> +
> +#define __TRACEPOINT_ENTRY(name) \
> + static tracepoint_ptr_t __tracepoint_ptr_##name __used \
> + __attribute__((section("__tracepoints_ptrs"))) = \
> + &__tracepoint_##name
> +#endif
> +
> #endif /* _LINUX_TRACEPOINT_H */
>
> /*
> @@ -253,19 +276,6 @@ extern void syscall_unregfunc(void);
> return static_key_false(&__tracepoint_##name.key); \
> }
>
> -#ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
> -#define __TRACEPOINT_ENTRY(name) \
> - asm(" .section \"__tracepoints_ptrs\", \"a\" \n" \
> - " .balign 4 \n" \
> - " .long __tracepoint_" #name " - . \n" \
> - " .previous \n")
> -#else
> -#define __TRACEPOINT_ENTRY(name) \
> - static struct tracepoint * const __tracepoint_ptr_##name __used \
> - __attribute__((section("__tracepoints_ptrs"))) = \
> - &__tracepoint_##name
> -#endif
> -
> /*
> * We have no guarantee that gcc and the linker won't up-align the tracepoint
> * structures, so we create an array of pointers that will be used for iteration
> diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
> index bf2c06ef9afc..a3be42304485 100644
> --- a/kernel/tracepoint.c
> +++ b/kernel/tracepoint.c
> @@ -28,8 +28,8 @@
> #include <linux/sched/task.h>
> #include <linux/static_key.h>
>
> -extern struct tracepoint * const __start___tracepoints_ptrs[];
> -extern struct tracepoint * const __stop___tracepoints_ptrs[];
> +extern tracepoint_ptr_t __start___tracepoints_ptrs[];
> +extern tracepoint_ptr_t __stop___tracepoints_ptrs[];
>
> DEFINE_SRCU(tracepoint_srcu);
> EXPORT_SYMBOL_GPL(tracepoint_srcu);
> @@ -371,25 +371,17 @@ int tracepoint_probe_unregister(struct tracepoint *tp,
> void *probe, void *data)
> }
> EXPORT_SYMBOL_GPL(tracepoint_probe_unregister);
>
> -static void for_each_tracepoint_range(struct tracepoint * const *begin,
> - struct tracepoint * const *end,
> +static void for_each_tracepoint_range(
> + tracepoint_ptr_t *begin, tracepoint_ptr_t *end,
> void (*fct)(struct tracepoint *tp, void *priv),
> void *priv)
> {
> + tracepoint_ptr_t *iter;
> +
> if (!begin)
> return;
> -
> - if (IS_ENABLED(CONFIG_HAVE_ARCH_PREL32_RELOCATIONS)) {
> - const int *iter;
> -
> - for (iter = (const int *)begin; iter < (const int *)end; iter++)
> - fct(offset_to_ptr(iter), priv);
> - } else {
> - struct tracepoint * const *iter;
> -
> - for (iter = begin; iter < end; iter++)
> - fct(*iter, priv);
> - }
> + for (iter = begin; iter < end; iter++)
> + fct(tracepoint_ptr_deref(iter), priv);
> }
>
> #ifdef CONFIG_MODULES
> --
> 2.17.1

--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

2018-10-12 21:08:31

by Ard Biesheuvel

[permalink] [raw]

Subject: Re: [PATCH for 4.19] tracepoint: Fix: out-of-bound tracepoint array iteration

Hi Mathieu,

On 12 October 2018 at 22:05, Mathieu Desnoyers
<[email protected]> wrote:
> commit 46e0c9be206f ("kernel: tracepoints: add support for relative
> references") changes the layout of the __tracepoint_ptrs section on
> architectures supporting relative references. However, it does so
> without turning struct tracepoint * const into const int * elsewhere in
> the tracepoint code, which has the following side-effect:
>
> tracepoint_module_{coming,going} invoke
> tp_module_going_check_quiescent() with mod->tracepoints_ptrs
> as first argument, and computes the end address of the array
> for the second argument with:
>
> mod->tracepoints_ptrs + mod->num_tracepoints
>
> However, because the type of mod->tracepoint_ptrs in module.h
> has not been changed from pointer to int, it passes an end
> pointer which is twice larger than the array, causing out-of-bound
> array accesses.
>
> Fix this by introducing a new typedef: tracepoint_ptr_t, which
> is either "const int" on architectures that have PREL32 relocations,
> or "struct tracepoint * const" on architectures that does not have
> this feature.
>
> Also provide a new tracepoint_ptr_defer() static inline to
> encapsulate deferencing this type rather than duplicate code and
> ugly idefs within the for_each_tracepoint_range() implementation.
>

Apologies for the breakage. FWIW, this looks like the correct approach
to me (and mirrors what I did for initcalls in the same series)

> This issue appears in 4.19-rc kernels, and should ideally be fixed
> before the end of the rc cycle.
>

+1

> Signed-off-by: Mathieu Desnoyers <[email protected]>
> Link: http://lkml.kernel.org/r/[email protected]
> Cc: Michael Ellerman <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Steven Rostedt (VMware) <[email protected]>
> Cc: Ard Biesheuvel <[email protected]>
> Cc: Arnd Bergmann <[email protected]>
> Cc: Benjamin Herrenschmidt <[email protected]>
> Cc: Bjorn Helgaas <[email protected]>
> Cc: Catalin Marinas <[email protected]>
> Cc: James Morris <[email protected]>
> Cc: James Morris <[email protected]>
> Cc: Jessica Yu <[email protected]>
> Cc: Josh Poimboeuf <[email protected]>
> Cc: Kees Cook <[email protected]>
> Cc: Nicolas Pitre <[email protected]>
> Cc: Paul Mackerras <[email protected]>
> Cc: Petr Mladek <[email protected]>
> Cc: Russell King <[email protected]>
> Cc: "Serge E. Hallyn" <[email protected]>
> Cc: Sergey Senozhatsky <[email protected]>
> Cc: Thomas Garnier <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Will Deacon <[email protected]>
> Cc: Andrew Morton <[email protected]>
> Cc: Linus Torvalds <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>

Acked-by: Ard Biesheuvel <[email protected]>

> ---
> include/linux/module.h | 2 +-
> include/linux/tracepoint-defs.h | 6 ++++++
> include/linux/tracepoint.h | 36 +++++++++++++++++++++------------
> kernel/tracepoint.c | 24 ++++++++--------------
> 4 files changed, 38 insertions(+), 30 deletions(-)
>
> diff --git a/include/linux/module.h b/include/linux/module.h
> index f807f15bebbe..cdab2451d6be 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -430,7 +430,7 @@ struct module {
>
> #ifdef CONFIG_TRACEPOINTS
> unsigned int num_tracepoints;
> - struct tracepoint * const *tracepoints_ptrs;
> + tracepoint_ptr_t *tracepoints_ptrs;
> #endif
> #ifdef HAVE_JUMP_LABEL
> struct jump_entry *jump_entries;
> diff --git a/include/linux/tracepoint-defs.h b/include/linux/tracepoint-defs.h
> index 22c5a46e9693..49ba9cde7e4b 100644
> --- a/include/linux/tracepoint-defs.h
> +++ b/include/linux/tracepoint-defs.h
> @@ -35,6 +35,12 @@ struct tracepoint {
> struct tracepoint_func __rcu *funcs;
> };
>
> +#ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
> +typedef const int tracepoint_ptr_t;
> +#else
> +typedef struct tracepoint * const tracepoint_ptr_t;
> +#endif
> +
> struct bpf_raw_event_map {
> struct tracepoint *tp;
> void *bpf_func;
> diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
> index 041f7e56a289..538ba1a58f5b 100644
> --- a/include/linux/tracepoint.h
> +++ b/include/linux/tracepoint.h
> @@ -99,6 +99,29 @@ extern void syscall_unregfunc(void);
> #define TRACE_DEFINE_ENUM(x)
> #define TRACE_DEFINE_SIZEOF(x)
>
> +#ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
> +static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
> +{
> + return offset_to_ptr(p);
> +}
> +
> +#define __TRACEPOINT_ENTRY(name) \
> + asm(" .section \"__tracepoints_ptrs\", \"a\" \n" \
> + " .balign 4 \n" \
> + " .long __tracepoint_" #name " - . \n" \
> + " .previous \n")
> +#else
> +static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
> +{
> + return *p;
> +}
> +
> +#define __TRACEPOINT_ENTRY(name) \
> + static tracepoint_ptr_t __tracepoint_ptr_##name __used \
> + __attribute__((section("__tracepoints_ptrs"))) = \
> + &__tracepoint_##name
> +#endif
> +
> #endif /* _LINUX_TRACEPOINT_H */
>
> /*
> @@ -253,19 +276,6 @@ extern void syscall_unregfunc(void);
> return static_key_false(&__tracepoint_##name.key); \
> }
>
> -#ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
> -#define __TRACEPOINT_ENTRY(name) \
> - asm(" .section \"__tracepoints_ptrs\", \"a\" \n" \
> - " .balign 4 \n" \
> - " .long __tracepoint_" #name " - . \n" \
> - " .previous \n")
> -#else
> -#define __TRACEPOINT_ENTRY(name) \
> - static struct tracepoint * const __tracepoint_ptr_##name __used \
> - __attribute__((section("__tracepoints_ptrs"))) = \
> - &__tracepoint_##name
> -#endif
> -
> /*
> * We have no guarantee that gcc and the linker won't up-align the tracepoint
> * structures, so we create an array of pointers that will be used for iteration
> diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
> index bf2c06ef9afc..a3be42304485 100644
> --- a/kernel/tracepoint.c
> +++ b/kernel/tracepoint.c
> @@ -28,8 +28,8 @@
> #include <linux/sched/task.h>
> #include <linux/static_key.h>
>
> -extern struct tracepoint * const __start___tracepoints_ptrs[];
> -extern struct tracepoint * const __stop___tracepoints_ptrs[];
> +extern tracepoint_ptr_t __start___tracepoints_ptrs[];
> +extern tracepoint_ptr_t __stop___tracepoints_ptrs[];
>
> DEFINE_SRCU(tracepoint_srcu);
> EXPORT_SYMBOL_GPL(tracepoint_srcu);
> @@ -371,25 +371,17 @@ int tracepoint_probe_unregister(struct tracepoint *tp, void *probe, void *data)
> }
> EXPORT_SYMBOL_GPL(tracepoint_probe_unregister);
>
> -static void for_each_tracepoint_range(struct tracepoint * const *begin,
> - struct tracepoint * const *end,
> +static void for_each_tracepoint_range(
> + tracepoint_ptr_t *begin, tracepoint_ptr_t *end,
> void (*fct)(struct tracepoint *tp, void *priv),
> void *priv)
> {
> + tracepoint_ptr_t *iter;
> +
> if (!begin)
> return;
> -
> - if (IS_ENABLED(CONFIG_HAVE_ARCH_PREL32_RELOCATIONS)) {
> - const int *iter;
> -
> - for (iter = (const int *)begin; iter < (const int *)end; iter++)
> - fct(offset_to_ptr(iter), priv);
> - } else {
> - struct tracepoint * const *iter;
> -
> - for (iter = begin; iter < end; iter++)
> - fct(*iter, priv);
> - }
> + for (iter = begin; iter < end; iter++)
> + fct(tracepoint_ptr_deref(iter), priv);
> }
>
> #ifdef CONFIG_MODULES
> --
> 2.17.1
>

2018-10-13 02:12:47

by kernel test robot

[permalink] [raw]

Subject: Re: [PATCH for 4.19] tracepoint: Fix: out-of-bound tracepoint array iteration

Attachments:

(No filename) (1.97 kB)
.config.gz (65.05 kB)
Download all attachments

2018-10-13 09:45:02

by kernel test robot

[permalink] [raw]

Subject: Re: [PATCH for 4.19] tracepoint: Fix: out-of-bound tracepoint array iteration

Hi Mathieu,

I love your patch! Yet something to improve:

[auto build test ERROR on tip/perf/core]
[also build test ERROR on v4.19-rc7 next-20181012]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Mathieu-Desnoyers/tracepoint-Fix-out-of-bound-tracepoint-array-iteration/20181013-073410
config: arm-allyesconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.2.0 make.cross ARCH=arm

All errors (new ones prefixed by >>):

In file included from kernel/tracepoint.c:18:0:
include/linux/module.h:433:2: error: unknown type name 'tracepoint_ptr_t'
tracepoint_ptr_t *tracepoints_ptrs;
^~~~~~~~~~~~~~~~
kernel/tracepoint.c: In function 'tracepoint_module_going':
>> kernel/tracepoint.c:504:30: error: passing argument 1 of 'for_each_tracepoint_range' from incompatible pointer type [-Werror=incompatible-pointer-types]
for_each_tracepoint_range(mod->tracepoints_ptrs,
^~~
kernel/tracepoint.c:374:13: note: expected 'struct tracepoint * const*' but argument is of type 'int *'
static void for_each_tracepoint_range(
^~~~~~~~~~~~~~~~~~~~~~~~~
kernel/tracepoint.c:505:5: error: passing argument 2 of 'for_each_tracepoint_range' from incompatible pointer type [-Werror=incompatible-pointer-types]
mod->tracepoints_ptrs + mod->num_tracepoints,
^~~
kernel/tracepoint.c:374:13: note: expected 'struct tracepoint * const*' but argument is of type 'int *'
static void for_each_tracepoint_range(
^~~~~~~~~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors

vim +/for_each_tracepoint_range +504 kernel/tracepoint.c

b75ef8b44 Mathieu Desnoyers 2011-08-10 485
de7b29739 Mathieu Desnoyers 2014-04-08 486 static void tracepoint_module_going(struct module *mod)
b75ef8b44 Mathieu Desnoyers 2011-08-10 487 {
de7b29739 Mathieu Desnoyers 2014-04-08 488 struct tp_module *tp_mod;
b75ef8b44 Mathieu Desnoyers 2011-08-10 489
7dec935a3 Steven Rostedt (Red Hat 2014-02-26 490) if (!mod->num_tracepoints)
de7b29739 Mathieu Desnoyers 2014-04-08 491 return;
7dec935a3 Steven Rostedt (Red Hat 2014-02-26 492)
de7b29739 Mathieu Desnoyers 2014-04-08 493 mutex_lock(&tracepoint_module_list_mutex);
de7b29739 Mathieu Desnoyers 2014-04-08 494 list_for_each_entry(tp_mod, &tracepoint_module_list, list) {
eb7d035c5 Steven Rostedt (Red Hat 2014-04-08 495) if (tp_mod->mod == mod) {
de7b29739 Mathieu Desnoyers 2014-04-08 496 blocking_notifier_call_chain(&tracepoint_notify_list,
de7b29739 Mathieu Desnoyers 2014-04-08 497 MODULE_STATE_GOING, tp_mod);
de7b29739 Mathieu Desnoyers 2014-04-08 498 list_del(&tp_mod->list);
de7b29739 Mathieu Desnoyers 2014-04-08 499 kfree(tp_mod);
de7b29739 Mathieu Desnoyers 2014-04-08 500 /*
de7b29739 Mathieu Desnoyers 2014-04-08 501 * Called the going notifier before checking for
de7b29739 Mathieu Desnoyers 2014-04-08 502 * quiescence.
de7b29739 Mathieu Desnoyers 2014-04-08 503 */
46e0c9be2 Ard Biesheuvel 2018-08-21 @504 for_each_tracepoint_range(mod->tracepoints_ptrs,
46e0c9be2 Ard Biesheuvel 2018-08-21 505 mod->tracepoints_ptrs + mod->num_tracepoints,
46e0c9be2 Ard Biesheuvel 2018-08-21 506 tp_module_going_check_quiescent, NULL);
b75ef8b44 Mathieu Desnoyers 2011-08-10 507 break;
b75ef8b44 Mathieu Desnoyers 2011-08-10 508 }
b75ef8b44 Mathieu Desnoyers 2011-08-10 509 }
b75ef8b44 Mathieu Desnoyers 2011-08-10 510 /*
b75ef8b44 Mathieu Desnoyers 2011-08-10 511 * In the case of modules that were tainted at "coming", we'll simply
b75ef8b44 Mathieu Desnoyers 2011-08-10 512 * walk through the list without finding it. We cannot use the "tainted"
b75ef8b44 Mathieu Desnoyers 2011-08-10 513 * flag on "going", in case a module taints the kernel only after being
b75ef8b44 Mathieu Desnoyers 2011-08-10 514 * loaded.
b75ef8b44 Mathieu Desnoyers 2011-08-10 515 */
de7b29739 Mathieu Desnoyers 2014-04-08 516 mutex_unlock(&tracepoint_module_list_mutex);
b75ef8b44 Mathieu Desnoyers 2011-08-10 517 }
227a83756 Ingo Molnar 2008-11-16 518

:::::: The code at line 504 was first introduced by commit
:::::: 46e0c9be206fa7b11aca75da2d6b8535d0139752 kernel: tracepoints: add support for relative references

:::::: TO: Ard Biesheuvel <[email protected]>
:::::: CC: Linus Torvalds <[email protected]>

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

Attachments:

(No filename) (5.10 kB)
.config.gz (65.05 kB)
Download all attachments

2018-10-13 15:24:59

by Ard Biesheuvel

[permalink] [raw]

Subject: Re: [PATCH for 4.19] tracepoint: Fix: out-of-bound tracepoint array iteration

On 12 October 2018 at 23:07, Ard Biesheuvel <[email protected]> wrote:
> Hi Mathieu,
>
> On 12 October 2018 at 22:05, Mathieu Desnoyers
> <[email protected]> wrote:
>> commit 46e0c9be206f ("kernel: tracepoints: add support for relative
>> references") changes the layout of the __tracepoint_ptrs section on
>> architectures supporting relative references. However, it does so
>> without turning struct tracepoint * const into const int * elsewhere in
>> the tracepoint code, which has the following side-effect:
>>
>> tracepoint_module_{coming,going} invoke
>> tp_module_going_check_quiescent() with mod->tracepoints_ptrs
>> as first argument, and computes the end address of the array
>> for the second argument with:
>>
>> mod->tracepoints_ptrs + mod->num_tracepoints
>>
>> However, because the type of mod->tracepoint_ptrs in module.h
>> has not been changed from pointer to int, it passes an end
>> pointer which is twice larger than the array, causing out-of-bound
>> array accesses.
>>
>> Fix this by introducing a new typedef: tracepoint_ptr_t, which
>> is either "const int" on architectures that have PREL32 relocations,
>> or "struct tracepoint * const" on architectures that does not have
>> this feature.
>>
>> Also provide a new tracepoint_ptr_defer() static inline to
>> encapsulate deferencing this type rather than duplicate code and
>> ugly idefs within the for_each_tracepoint_range() implementation.
>>
>
> Apologies for the breakage. FWIW, this looks like the correct approach
> to me (and mirrors what I did for initcalls in the same series)
>
>> This issue appears in 4.19-rc kernels, and should ideally be fixed
>> before the end of the rc cycle.
>>
>
> +1
>
>> Signed-off-by: Mathieu Desnoyers <[email protected]>
>> Link: http://lkml.kernel.org/r/[email protected]
>> Cc: Michael Ellerman <[email protected]>
>> Cc: Ingo Molnar <[email protected]>
>> Cc: Steven Rostedt (VMware) <[email protected]>
>> Cc: Ard Biesheuvel <[email protected]>
>> Cc: Arnd Bergmann <[email protected]>
>> Cc: Benjamin Herrenschmidt <[email protected]>
>> Cc: Bjorn Helgaas <[email protected]>
>> Cc: Catalin Marinas <[email protected]>
>> Cc: James Morris <[email protected]>
>> Cc: James Morris <[email protected]>
>> Cc: Jessica Yu <[email protected]>
>> Cc: Josh Poimboeuf <[email protected]>
>> Cc: Kees Cook <[email protected]>
>> Cc: Nicolas Pitre <[email protected]>
>> Cc: Paul Mackerras <[email protected]>
>> Cc: Petr Mladek <[email protected]>
>> Cc: Russell King <[email protected]>
>> Cc: "Serge E. Hallyn" <[email protected]>
>> Cc: Sergey Senozhatsky <[email protected]>
>> Cc: Thomas Garnier <[email protected]>
>> Cc: Thomas Gleixner <[email protected]>
>> Cc: Will Deacon <[email protected]>
>> Cc: Andrew Morton <[email protected]>
>> Cc: Linus Torvalds <[email protected]>
>> Cc: Greg Kroah-Hartman <[email protected]>
>
> Acked-by: Ard Biesheuvel <[email protected]>
>

This fixes the build breakage for me that kbuild test robot reports.

diff --git a/include/linux/module.h b/include/linux/module.h
index cdab2451d6be..e19ae08c7fb8 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -20,6 +20,7 @@
#include <linux/export.h>
#include <linux/rbtree_latch.h>
#include <linux/error-injection.h>
+#include <linux/tracepoint-defs.h>

#include <linux/percpu.h>
#include <asm/module.h>

>> ---
>> include/linux/module.h | 2 +-
>> include/linux/tracepoint-defs.h | 6 ++++++
>> include/linux/tracepoint.h | 36 +++++++++++++++++++++------------
>> kernel/tracepoint.c | 24 ++++++++--------------
>> 4 files changed, 38 insertions(+), 30 deletions(-)
>>
>> diff --git a/include/linux/module.h b/include/linux/module.h
>> index f807f15bebbe..cdab2451d6be 100644
>> --- a/include/linux/module.h
>> +++ b/include/linux/module.h
>> @@ -430,7 +430,7 @@ struct module {
>>
>> #ifdef CONFIG_TRACEPOINTS
>> unsigned int num_tracepoints;
>> - struct tracepoint * const *tracepoints_ptrs;
>> + tracepoint_ptr_t *tracepoints_ptrs;
>> #endif
>> #ifdef HAVE_JUMP_LABEL
>> struct jump_entry *jump_entries;
>> diff --git a/include/linux/tracepoint-defs.h b/include/linux/tracepoint-defs.h
>> index 22c5a46e9693..49ba9cde7e4b 100644
>> --- a/include/linux/tracepoint-defs.h
>> +++ b/include/linux/tracepoint-defs.h
>> @@ -35,6 +35,12 @@ struct tracepoint {
>> struct tracepoint_func __rcu *funcs;
>> };
>>
>> +#ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
>> +typedef const int tracepoint_ptr_t;
>> +#else
>> +typedef struct tracepoint * const tracepoint_ptr_t;
>> +#endif
>> +
>> struct bpf_raw_event_map {
>> struct tracepoint *tp;
>> void *bpf_func;
>> diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
>> index 041f7e56a289..538ba1a58f5b 100644
>> --- a/include/linux/tracepoint.h
>> +++ b/include/linux/tracepoint.h
>> @@ -99,6 +99,29 @@ extern void syscall_unregfunc(void);
>> #define TRACE_DEFINE_ENUM(x)
>> #define TRACE_DEFINE_SIZEOF(x)
>>
>> +#ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
>> +static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
>> +{
>> + return offset_to_ptr(p);
>> +}
>> +
>> +#define __TRACEPOINT_ENTRY(name) \
>> + asm(" .section \"__tracepoints_ptrs\", \"a\" \n" \
>> + " .balign 4 \n" \
>> + " .long __tracepoint_" #name " - . \n" \
>> + " .previous \n")
>> +#else
>> +static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
>> +{
>> + return *p;
>> +}
>> +
>> +#define __TRACEPOINT_ENTRY(name) \
>> + static tracepoint_ptr_t __tracepoint_ptr_##name __used \
>> + __attribute__((section("__tracepoints_ptrs"))) = \
>> + &__tracepoint_##name
>> +#endif
>> +
>> #endif /* _LINUX_TRACEPOINT_H */
>>
>> /*
>> @@ -253,19 +276,6 @@ extern void syscall_unregfunc(void);
>> return static_key_false(&__tracepoint_##name.key); \
>> }
>>
>> -#ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
>> -#define __TRACEPOINT_ENTRY(name) \
>> - asm(" .section \"__tracepoints_ptrs\", \"a\" \n" \
>> - " .balign 4 \n" \
>> - " .long __tracepoint_" #name " - . \n" \
>> - " .previous \n")
>> -#else
>> -#define __TRACEPOINT_ENTRY(name) \
>> - static struct tracepoint * const __tracepoint_ptr_##name __used \
>> - __attribute__((section("__tracepoints_ptrs"))) = \
>> - &__tracepoint_##name
>> -#endif
>> -
>> /*
>> * We have no guarantee that gcc and the linker won't up-align the tracepoint
>> * structures, so we create an array of pointers that will be used for iteration
>> diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
>> index bf2c06ef9afc..a3be42304485 100644
>> --- a/kernel/tracepoint.c
>> +++ b/kernel/tracepoint.c
>> @@ -28,8 +28,8 @@
>> #include <linux/sched/task.h>
>> #include <linux/static_key.h>
>>
>> -extern struct tracepoint * const __start___tracepoints_ptrs[];
>> -extern struct tracepoint * const __stop___tracepoints_ptrs[];
>> +extern tracepoint_ptr_t __start___tracepoints_ptrs[];
>> +extern tracepoint_ptr_t __stop___tracepoints_ptrs[];
>>
>> DEFINE_SRCU(tracepoint_srcu);
>> EXPORT_SYMBOL_GPL(tracepoint_srcu);
>> @@ -371,25 +371,17 @@ int tracepoint_probe_unregister(struct tracepoint *tp, void *probe, void *data)
>> }
>> EXPORT_SYMBOL_GPL(tracepoint_probe_unregister);
>>
>> -static void for_each_tracepoint_range(struct tracepoint * const *begin,
>> - struct tracepoint * const *end,
>> +static void for_each_tracepoint_range(
>> + tracepoint_ptr_t *begin, tracepoint_ptr_t *end,
>> void (*fct)(struct tracepoint *tp, void *priv),
>> void *priv)
>> {
>> + tracepoint_ptr_t *iter;
>> +
>> if (!begin)
>> return;
>> -
>> - if (IS_ENABLED(CONFIG_HAVE_ARCH_PREL32_RELOCATIONS)) {
>> - const int *iter;
>> -
>> - for (iter = (const int *)begin; iter < (const int *)end; iter++)
>> - fct(offset_to_ptr(iter), priv);
>> - } else {
>> - struct tracepoint * const *iter;
>> -
>> - for (iter = begin; iter < end; iter++)
>> - fct(*iter, priv);
>> - }
>> + for (iter = begin; iter < end; iter++)
>> + fct(tracepoint_ptr_deref(iter), priv);
>> }
>>
>> #ifdef CONFIG_MODULES
>> --
>> 2.17.1
>>

2018-10-13 18:35:06

by Mathieu Desnoyers

[permalink] [raw]

Subject: Re: [PATCH for 4.19] tracepoint: Fix: out-of-bound tracepoint array iteration

----- On Oct 13, 2018, at 11:24 AM, Ard Biesheuvel [email protected] wrote:

> On 12 October 2018 at 23:07, Ard Biesheuvel <[email protected]> wrote:
>> Hi Mathieu,
>>
>> On 12 October 2018 at 22:05, Mathieu Desnoyers
>> <[email protected]> wrote:
>>> commit 46e0c9be206f ("kernel: tracepoints: add support for relative
>>> references") changes the layout of the __tracepoint_ptrs section on
>>> architectures supporting relative references. However, it does so
>>> without turning struct tracepoint * const into const int * elsewhere in
>>> the tracepoint code, which has the following side-effect:
>>>
>>> tracepoint_module_{coming,going} invoke
>>> tp_module_going_check_quiescent() with mod->tracepoints_ptrs
>>> as first argument, and computes the end address of the array
>>> for the second argument with:
>>>
>>> mod->tracepoints_ptrs + mod->num_tracepoints
>>>
>>> However, because the type of mod->tracepoint_ptrs in module.h
>>> has not been changed from pointer to int, it passes an end
>>> pointer which is twice larger than the array, causing out-of-bound
>>> array accesses.
>>>
>>> Fix this by introducing a new typedef: tracepoint_ptr_t, which
>>> is either "const int" on architectures that have PREL32 relocations,
>>> or "struct tracepoint * const" on architectures that does not have
>>> this feature.
>>>
>>> Also provide a new tracepoint_ptr_defer() static inline to
>>> encapsulate deferencing this type rather than duplicate code and
>>> ugly idefs within the for_each_tracepoint_range() implementation.
>>>
>>
>> Apologies for the breakage. FWIW, this looks like the correct approach
>> to me (and mirrors what I did for initcalls in the same series)
>>
>>> This issue appears in 4.19-rc kernels, and should ideally be fixed
>>> before the end of the rc cycle.
>>>
>>
>> +1
>>
>>> Signed-off-by: Mathieu Desnoyers <[email protected]>
>>> Link: http://lkml.kernel.org/r/[email protected]
>>> Cc: Michael Ellerman <[email protected]>
>>> Cc: Ingo Molnar <[email protected]>
>>> Cc: Steven Rostedt (VMware) <[email protected]>
>>> Cc: Ard Biesheuvel <[email protected]>
>>> Cc: Arnd Bergmann <[email protected]>
>>> Cc: Benjamin Herrenschmidt <[email protected]>
>>> Cc: Bjorn Helgaas <[email protected]>
>>> Cc: Catalin Marinas <[email protected]>
>>> Cc: James Morris <[email protected]>
>>> Cc: James Morris <[email protected]>
>>> Cc: Jessica Yu <[email protected]>
>>> Cc: Josh Poimboeuf <[email protected]>
>>> Cc: Kees Cook <[email protected]>
>>> Cc: Nicolas Pitre <[email protected]>
>>> Cc: Paul Mackerras <[email protected]>
>>> Cc: Petr Mladek <[email protected]>
>>> Cc: Russell King <[email protected]>
>>> Cc: "Serge E. Hallyn" <[email protected]>
>>> Cc: Sergey Senozhatsky <[email protected]>
>>> Cc: Thomas Garnier <[email protected]>
>>> Cc: Thomas Gleixner <[email protected]>
>>> Cc: Will Deacon <[email protected]>
>>> Cc: Andrew Morton <[email protected]>
>>> Cc: Linus Torvalds <[email protected]>
>>> Cc: Greg Kroah-Hartman <[email protected]>
>>
>> Acked-by: Ard Biesheuvel <[email protected]>
>>
>
> This fixes the build breakage for me that kbuild test robot reports.
>
> diff --git a/include/linux/module.h b/include/linux/module.h
> index cdab2451d6be..e19ae08c7fb8 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -20,6 +20,7 @@
> #include <linux/export.h>
> #include <linux/rbtree_latch.h>
> #include <linux/error-injection.h>
> +#include <linux/tracepoint-defs.h>

You've beat me to it :) I'll fold this change in a v2 of the patch,

Thanks!

Mathieu

>
> #include <linux/percpu.h>
> #include <asm/module.h>
>
>
>
>
>
>>> ---
>>> include/linux/module.h | 2 +-
>>> include/linux/tracepoint-defs.h | 6 ++++++
>>> include/linux/tracepoint.h | 36 +++++++++++++++++++++------------
>>> kernel/tracepoint.c | 24 ++++++++--------------
>>> 4 files changed, 38 insertions(+), 30 deletions(-)
>>>
>>> diff --git a/include/linux/module.h b/include/linux/module.h
>>> index f807f15bebbe..cdab2451d6be 100644
>>> --- a/include/linux/module.h
>>> +++ b/include/linux/module.h
>>> @@ -430,7 +430,7 @@ struct module {
>>>
>>> #ifdef CONFIG_TRACEPOINTS
>>> unsigned int num_tracepoints;
>>> - struct tracepoint * const *tracepoints_ptrs;
>>> + tracepoint_ptr_t *tracepoints_ptrs;
>>> #endif
>>> #ifdef HAVE_JUMP_LABEL
>>> struct jump_entry *jump_entries;
>>> diff --git a/include/linux/tracepoint-defs.h b/include/linux/tracepoint-defs.h
>>> index 22c5a46e9693..49ba9cde7e4b 100644
>>> --- a/include/linux/tracepoint-defs.h
>>> +++ b/include/linux/tracepoint-defs.h
>>> @@ -35,6 +35,12 @@ struct tracepoint {
>>> struct tracepoint_func __rcu *funcs;
>>> };
>>>
>>> +#ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
>>> +typedef const int tracepoint_ptr_t;
>>> +#else
>>> +typedef struct tracepoint * const tracepoint_ptr_t;
>>> +#endif
>>> +
>>> struct bpf_raw_event_map {
>>> struct tracepoint *tp;
>>> void *bpf_func;
>>> diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
>>> index 041f7e56a289..538ba1a58f5b 100644
>>> --- a/include/linux/tracepoint.h
>>> +++ b/include/linux/tracepoint.h
>>> @@ -99,6 +99,29 @@ extern void syscall_unregfunc(void);
>>> #define TRACE_DEFINE_ENUM(x)
>>> #define TRACE_DEFINE_SIZEOF(x)
>>>
>>> +#ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
>>> +static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
>>> +{
>>> + return offset_to_ptr(p);
>>> +}
>>> +
>>> +#define __TRACEPOINT_ENTRY(name) \
>>> + asm(" .section \"__tracepoints_ptrs\", \"a\" \n" \
>>> + " .balign 4 \n" \
>>> + " .long __tracepoint_" #name " - . \n" \
>>> + " .previous \n")
>>> +#else
>>> +static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
>>> +{
>>> + return *p;
>>> +}
>>> +
>>> +#define __TRACEPOINT_ENTRY(name) \
>>> + static tracepoint_ptr_t __tracepoint_ptr_##name __used \
>>> + __attribute__((section("__tracepoints_ptrs"))) = \
>>> + &__tracepoint_##name
>>> +#endif
>>> +
>>> #endif /* _LINUX_TRACEPOINT_H */
>>>
>>> /*
>>> @@ -253,19 +276,6 @@ extern void syscall_unregfunc(void);
>>> return static_key_false(&__tracepoint_##name.key); \
>>> }
>>>
>>> -#ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
>>> -#define __TRACEPOINT_ENTRY(name) \
>>> - asm(" .section \"__tracepoints_ptrs\", \"a\" \n" \
>>> - " .balign 4 \n" \
>>> - " .long __tracepoint_" #name " - . \n" \
>>> - " .previous \n")
>>> -#else
>>> -#define __TRACEPOINT_ENTRY(name) \
>>> - static struct tracepoint * const __tracepoint_ptr_##name __used \
>>> - __attribute__((section("__tracepoints_ptrs"))) = \
>>> - &__tracepoint_##name
>>> -#endif
>>> -
>>> /*
>>> * We have no guarantee that gcc and the linker won't up-align the tracepoint
>>> * structures, so we create an array of pointers that will be used for iteration
>>> diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
>>> index bf2c06ef9afc..a3be42304485 100644
>>> --- a/kernel/tracepoint.c
>>> +++ b/kernel/tracepoint.c
>>> @@ -28,8 +28,8 @@
>>> #include <linux/sched/task.h>
>>> #include <linux/static_key.h>
>>>
>>> -extern struct tracepoint * const __start___tracepoints_ptrs[];
>>> -extern struct tracepoint * const __stop___tracepoints_ptrs[];
>>> +extern tracepoint_ptr_t __start___tracepoints_ptrs[];
>>> +extern tracepoint_ptr_t __stop___tracepoints_ptrs[];
>>>
>>> DEFINE_SRCU(tracepoint_srcu);
>>> EXPORT_SYMBOL_GPL(tracepoint_srcu);
>>> @@ -371,25 +371,17 @@ int tracepoint_probe_unregister(struct tracepoint *tp,
>>> void *probe, void *data)
>>> }
>>> EXPORT_SYMBOL_GPL(tracepoint_probe_unregister);
>>>
>>> -static void for_each_tracepoint_range(struct tracepoint * const *begin,
>>> - struct tracepoint * const *end,
>>> +static void for_each_tracepoint_range(
>>> + tracepoint_ptr_t *begin, tracepoint_ptr_t *end,
>>> void (*fct)(struct tracepoint *tp, void *priv),
>>> void *priv)
>>> {
>>> + tracepoint_ptr_t *iter;
>>> +
>>> if (!begin)
>>> return;
>>> -
>>> - if (IS_ENABLED(CONFIG_HAVE_ARCH_PREL32_RELOCATIONS)) {
>>> - const int *iter;
>>> -
>>> - for (iter = (const int *)begin; iter < (const int *)end; iter++)
>>> - fct(offset_to_ptr(iter), priv);
>>> - } else {
>>> - struct tracepoint * const *iter;
>>> -
>>> - for (iter = begin; iter < end; iter++)
>>> - fct(*iter, priv);
>>> - }
>>> + for (iter = begin; iter < end; iter++)
>>> + fct(tracepoint_ptr_deref(iter), priv);
>>> }
>>>
>>> #ifdef CONFIG_MODULES
>>> --
>>> 2.17.1

--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

2018-10-13 19:06:56

by Mathieu Desnoyers

[permalink] [raw]

Subject: Re: [PATCH for 4.19] tracepoint: Fix: out-of-bound tracepoint array iteration

----- On Oct 13, 2018, at 2:34 PM, Mathieu Desnoyers [email protected] wrote:

> ----- On Oct 13, 2018, at 11:24 AM, Ard Biesheuvel [email protected]
> wrote:
>
>> On 12 October 2018 at 23:07, Ard Biesheuvel <[email protected]> wrote:
>>> Hi Mathieu,
>>>
>>> On 12 October 2018 at 22:05, Mathieu Desnoyers
>>> <[email protected]> wrote:
>>>> commit 46e0c9be206f ("kernel: tracepoints: add support for relative
>>>> references") changes the layout of the __tracepoint_ptrs section on
>>>> architectures supporting relative references. However, it does so
>>>> without turning struct tracepoint * const into const int * elsewhere in
>>>> the tracepoint code, which has the following side-effect:
>>>>
>>>> tracepoint_module_{coming,going} invoke
>>>> tp_module_going_check_quiescent() with mod->tracepoints_ptrs
>>>> as first argument, and computes the end address of the array
>>>> for the second argument with:
>>>>
>>>> mod->tracepoints_ptrs + mod->num_tracepoints
>>>>
>>>> However, because the type of mod->tracepoint_ptrs in module.h
>>>> has not been changed from pointer to int, it passes an end
>>>> pointer which is twice larger than the array, causing out-of-bound
>>>> array accesses.
>>>>
>>>> Fix this by introducing a new typedef: tracepoint_ptr_t, which
>>>> is either "const int" on architectures that have PREL32 relocations,
>>>> or "struct tracepoint * const" on architectures that does not have
>>>> this feature.
>>>>
>>>> Also provide a new tracepoint_ptr_defer() static inline to
>>>> encapsulate deferencing this type rather than duplicate code and
>>>> ugly idefs within the for_each_tracepoint_range() implementation.
>>>>
>>>
>>> Apologies for the breakage. FWIW, this looks like the correct approach
>>> to me (and mirrors what I did for initcalls in the same series)
>>>
>>>> This issue appears in 4.19-rc kernels, and should ideally be fixed
>>>> before the end of the rc cycle.
>>>>
>>>
>>> +1
>>>
>>>> Signed-off-by: Mathieu Desnoyers <[email protected]>
>>>> Link: http://lkml.kernel.org/r/[email protected]
>>>> Cc: Michael Ellerman <[email protected]>
>>>> Cc: Ingo Molnar <[email protected]>
>>>> Cc: Steven Rostedt (VMware) <[email protected]>
>>>> Cc: Ard Biesheuvel <[email protected]>
>>>> Cc: Arnd Bergmann <[email protected]>
>>>> Cc: Benjamin Herrenschmidt <[email protected]>
>>>> Cc: Bjorn Helgaas <[email protected]>
>>>> Cc: Catalin Marinas <[email protected]>
>>>> Cc: James Morris <[email protected]>
>>>> Cc: James Morris <[email protected]>
>>>> Cc: Jessica Yu <[email protected]>
>>>> Cc: Josh Poimboeuf <[email protected]>
>>>> Cc: Kees Cook <[email protected]>
>>>> Cc: Nicolas Pitre <[email protected]>
>>>> Cc: Paul Mackerras <[email protected]>
>>>> Cc: Petr Mladek <[email protected]>
>>>> Cc: Russell King <[email protected]>
>>>> Cc: "Serge E. Hallyn" <[email protected]>
>>>> Cc: Sergey Senozhatsky <[email protected]>
>>>> Cc: Thomas Garnier <[email protected]>
>>>> Cc: Thomas Gleixner <[email protected]>
>>>> Cc: Will Deacon <[email protected]>
>>>> Cc: Andrew Morton <[email protected]>
>>>> Cc: Linus Torvalds <[email protected]>
>>>> Cc: Greg Kroah-Hartman <[email protected]>
>>>
>>> Acked-by: Ard Biesheuvel <[email protected]>
>>>
>>
>> This fixes the build breakage for me that kbuild test robot reports.
>>
>> diff --git a/include/linux/module.h b/include/linux/module.h
>> index cdab2451d6be..e19ae08c7fb8 100644
>> --- a/include/linux/module.h
>> +++ b/include/linux/module.h
>> @@ -20,6 +20,7 @@
>> #include <linux/export.h>
>> #include <linux/rbtree_latch.h>
>> #include <linux/error-injection.h>
>> +#include <linux/tracepoint-defs.h>
>
> You've beat me to it :) I'll fold this change in a v2 of the patch,

Digging a bit deeper into module.c, I notice it's not really an
out-of-bound access that is generated by this issue, because
setting mod->num_tracepoints is done in by module.c like this:

mod->tracepoints_ptrs = section_objs(info, "__tracepoints_ptrs",
sizeof(*mod->tracepoints_ptrs),
&mod->num_tracepoints);

So basically, since sizeof(*mod->tracepoints_ptrs) is a pointer size
(rather than sizeof(int)), num_tracepoints is erroneously set to half the
size it should be on 64-bit arch. So we an odd number of tracepoints, we
lose the last tracepoint due to effect of integer division.

So in the module going notifier:

for_each_tracepoint_range(mod->tracepoints_ptrs,
mod->tracepoints_ptrs + mod->num_tracepoints,
tp_module_going_check_quiescent, NULL);

the expression (mod->tracepoints_ptrs + mod->num_tracepoints) actually
evaluates to something within the bounds of the array, but miss the
last tracepoint if the number of tracepoints is odd on 64-bit arch.

So I'll also update the patch changelog in v2. Given it does not change
the patch content, I'll keep your acked-by. Please let me know if you
spot anything.

Thanks,

Mathieu

>
> Thanks!
>
> Mathieu
>
>>
>> #include <linux/percpu.h>
>> #include <asm/module.h>
>>
>>
>>
>>
>>
>>>> ---
>>>> include/linux/module.h | 2 +-
>>>> include/linux/tracepoint-defs.h | 6 ++++++
>>>> include/linux/tracepoint.h | 36 +++++++++++++++++++++------------
>>>> kernel/tracepoint.c | 24 ++++++++--------------
>>>> 4 files changed, 38 insertions(+), 30 deletions(-)
>>>>
>>>> diff --git a/include/linux/module.h b/include/linux/module.h
>>>> index f807f15bebbe..cdab2451d6be 100644
>>>> --- a/include/linux/module.h
>>>> +++ b/include/linux/module.h
>>>> @@ -430,7 +430,7 @@ struct module {
>>>>
>>>> #ifdef CONFIG_TRACEPOINTS
>>>> unsigned int num_tracepoints;
>>>> - struct tracepoint * const *tracepoints_ptrs;
>>>> + tracepoint_ptr_t *tracepoints_ptrs;
>>>> #endif
>>>> #ifdef HAVE_JUMP_LABEL
>>>> struct jump_entry *jump_entries;
>>>> diff --git a/include/linux/tracepoint-defs.h b/include/linux/tracepoint-defs.h
>>>> index 22c5a46e9693..49ba9cde7e4b 100644
>>>> --- a/include/linux/tracepoint-defs.h
>>>> +++ b/include/linux/tracepoint-defs.h
>>>> @@ -35,6 +35,12 @@ struct tracepoint {
>>>> struct tracepoint_func __rcu *funcs;
>>>> };
>>>>
>>>> +#ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
>>>> +typedef const int tracepoint_ptr_t;
>>>> +#else
>>>> +typedef struct tracepoint * const tracepoint_ptr_t;
>>>> +#endif
>>>> +
>>>> struct bpf_raw_event_map {
>>>> struct tracepoint *tp;
>>>> void *bpf_func;
>>>> diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
>>>> index 041f7e56a289..538ba1a58f5b 100644
>>>> --- a/include/linux/tracepoint.h
>>>> +++ b/include/linux/tracepoint.h
>>>> @@ -99,6 +99,29 @@ extern void syscall_unregfunc(void);
>>>> #define TRACE_DEFINE_ENUM(x)
>>>> #define TRACE_DEFINE_SIZEOF(x)
>>>>
>>>> +#ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
>>>> +static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
>>>> +{
>>>> + return offset_to_ptr(p);
>>>> +}
>>>> +
>>>> +#define __TRACEPOINT_ENTRY(name) \
>>>> + asm(" .section \"__tracepoints_ptrs\", \"a\" \n" \
>>>> + " .balign 4 \n" \
>>>> + " .long __tracepoint_" #name " - . \n" \
>>>> + " .previous \n")
>>>> +#else
>>>> +static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
>>>> +{
>>>> + return *p;
>>>> +}
>>>> +
>>>> +#define __TRACEPOINT_ENTRY(name) \
>>>> + static tracepoint_ptr_t __tracepoint_ptr_##name __used \
>>>> + __attribute__((section("__tracepoints_ptrs"))) = \
>>>> + &__tracepoint_##name
>>>> +#endif
>>>> +
>>>> #endif /* _LINUX_TRACEPOINT_H */
>>>>
>>>> /*
>>>> @@ -253,19 +276,6 @@ extern void syscall_unregfunc(void);
>>>> return static_key_false(&__tracepoint_##name.key); \
>>>> }
>>>>
>>>> -#ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
>>>> -#define __TRACEPOINT_ENTRY(name) \
>>>> - asm(" .section \"__tracepoints_ptrs\", \"a\" \n" \
>>>> - " .balign 4 \n" \
>>>> - " .long __tracepoint_" #name " - . \n" \
>>>> - " .previous \n")
>>>> -#else
>>>> -#define __TRACEPOINT_ENTRY(name) \
>>>> - static struct tracepoint * const __tracepoint_ptr_##name __used \
>>>> - __attribute__((section("__tracepoints_ptrs"))) = \
>>>> - &__tracepoint_##name
>>>> -#endif
>>>> -
>>>> /*
>>>> * We have no guarantee that gcc and the linker won't up-align the tracepoint
>>>> * structures, so we create an array of pointers that will be used for iteration
>>>> diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
>>>> index bf2c06ef9afc..a3be42304485 100644
>>>> --- a/kernel/tracepoint.c
>>>> +++ b/kernel/tracepoint.c
>>>> @@ -28,8 +28,8 @@
>>>> #include <linux/sched/task.h>
>>>> #include <linux/static_key.h>
>>>>
>>>> -extern struct tracepoint * const __start___tracepoints_ptrs[];
>>>> -extern struct tracepoint * const __stop___tracepoints_ptrs[];
>>>> +extern tracepoint_ptr_t __start___tracepoints_ptrs[];
>>>> +extern tracepoint_ptr_t __stop___tracepoints_ptrs[];
>>>>
>>>> DEFINE_SRCU(tracepoint_srcu);
>>>> EXPORT_SYMBOL_GPL(tracepoint_srcu);
>>>> @@ -371,25 +371,17 @@ int tracepoint_probe_unregister(struct tracepoint *tp,
>>>> void *probe, void *data)
>>>> }
>>>> EXPORT_SYMBOL_GPL(tracepoint_probe_unregister);
>>>>
>>>> -static void for_each_tracepoint_range(struct tracepoint * const *begin,
>>>> - struct tracepoint * const *end,
>>>> +static void for_each_tracepoint_range(
>>>> + tracepoint_ptr_t *begin, tracepoint_ptr_t *end,
>>>> void (*fct)(struct tracepoint *tp, void *priv),
>>>> void *priv)
>>>> {
>>>> + tracepoint_ptr_t *iter;
>>>> +
>>>> if (!begin)
>>>> return;
>>>> -
>>>> - if (IS_ENABLED(CONFIG_HAVE_ARCH_PREL32_RELOCATIONS)) {
>>>> - const int *iter;
>>>> -
>>>> - for (iter = (const int *)begin; iter < (const int *)end; iter++)
>>>> - fct(offset_to_ptr(iter), priv);
>>>> - } else {
>>>> - struct tracepoint * const *iter;
>>>> -
>>>> - for (iter = begin; iter < end; iter++)
>>>> - fct(*iter, priv);
>>>> - }
>>>> + for (iter = begin; iter < end; iter++)
>>>> + fct(tracepoint_ptr_deref(iter), priv);
>>>> }
>>>>
>>>> #ifdef CONFIG_MODULES
>>>> --
>>>> 2.17.1
>
> --
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com

--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

2018-10-15 11:36:36

by Jessica Yu

[permalink] [raw]

Subject: Re: [PATCH for 4.19] tracepoint: Fix: out-of-bound tracepoint array iteration

+++ Mathieu Desnoyers [12/10/18 16:05 -0400]:
>commit 46e0c9be206f ("kernel: tracepoints: add support for relative
>references") changes the layout of the __tracepoint_ptrs section on
>architectures supporting relative references. However, it does so
>without turning struct tracepoint * const into const int * elsewhere in
>the tracepoint code, which has the following side-effect:
>
>tracepoint_module_{coming,going} invoke
>tp_module_going_check_quiescent() with mod->tracepoints_ptrs
>as first argument, and computes the end address of the array
>for the second argument with:
>
> mod->tracepoints_ptrs + mod->num_tracepoints
>
>However, because the type of mod->tracepoint_ptrs in module.h
>has not been changed from pointer to int, it passes an end
>pointer which is twice larger than the array, causing out-of-bound
>array accesses.
>
>Fix this by introducing a new typedef: tracepoint_ptr_t, which
>is either "const int" on architectures that have PREL32 relocations,
>or "struct tracepoint * const" on architectures that does not have
>this feature.
>
>Also provide a new tracepoint_ptr_defer() static inline to
>encapsulate deferencing this type rather than duplicate code and
>ugly idefs within the for_each_tracepoint_range() implementation.
>
>This issue appears in 4.19-rc kernels, and should ideally be fixed
>before the end of the rc cycle.
>
>Signed-off-by: Mathieu Desnoyers <[email protected]>
>Link: http://lkml.kernel.org/r/[email protected]
>Cc: Michael Ellerman <[email protected]>
>Cc: Ingo Molnar <[email protected]>
>Cc: Steven Rostedt (VMware) <[email protected]>
>Cc: Ard Biesheuvel <[email protected]>
>Cc: Arnd Bergmann <[email protected]>
>Cc: Benjamin Herrenschmidt <[email protected]>
>Cc: Bjorn Helgaas <[email protected]>
>Cc: Catalin Marinas <[email protected]>
>Cc: James Morris <[email protected]>
>Cc: James Morris <[email protected]>
>Cc: Jessica Yu <[email protected]>
>Cc: Josh Poimboeuf <[email protected]>
>Cc: Kees Cook <[email protected]>
>Cc: Nicolas Pitre <[email protected]>
>Cc: Paul Mackerras <[email protected]>
>Cc: Petr Mladek <[email protected]>
>Cc: Russell King <[email protected]>
>Cc: "Serge E. Hallyn" <[email protected]>
>Cc: Sergey Senozhatsky <[email protected]>
>Cc: Thomas Garnier <[email protected]>
>Cc: Thomas Gleixner <[email protected]>
>Cc: Will Deacon <[email protected]>
>Cc: Andrew Morton <[email protected]>
>Cc: Linus Torvalds <[email protected]>
>Cc: Greg Kroah-Hartman <[email protected]>

For module parts:

Acked-by: Jessica Yu <[email protected]>

>---
> include/linux/module.h | 2 +-
> include/linux/tracepoint-defs.h | 6 ++++++
> include/linux/tracepoint.h | 36 +++++++++++++++++++++------------
> kernel/tracepoint.c | 24 ++++++++--------------
> 4 files changed, 38 insertions(+), 30 deletions(-)
>
>diff --git a/include/linux/module.h b/include/linux/module.h
>index f807f15bebbe..cdab2451d6be 100644
>--- a/include/linux/module.h
>+++ b/include/linux/module.h
>@@ -430,7 +430,7 @@ struct module {
>
> #ifdef CONFIG_TRACEPOINTS
> unsigned int num_tracepoints;
>- struct tracepoint * const *tracepoints_ptrs;
>+ tracepoint_ptr_t *tracepoints_ptrs;
> #endif
> #ifdef HAVE_JUMP_LABEL
> struct jump_entry *jump_entries;
>diff --git a/include/linux/tracepoint-defs.h b/include/linux/tracepoint-defs.h
>index 22c5a46e9693..49ba9cde7e4b 100644
>--- a/include/linux/tracepoint-defs.h
>+++ b/include/linux/tracepoint-defs.h
>@@ -35,6 +35,12 @@ struct tracepoint {
> struct tracepoint_func __rcu *funcs;
> };
>
>+#ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
>+typedef const int tracepoint_ptr_t;
>+#else
>+typedef struct tracepoint * const tracepoint_ptr_t;
>+#endif
>+
> struct bpf_raw_event_map {
> struct tracepoint *tp;
> void *bpf_func;
>diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
>index 041f7e56a289..538ba1a58f5b 100644
>--- a/include/linux/tracepoint.h
>+++ b/include/linux/tracepoint.h
>@@ -99,6 +99,29 @@ extern void syscall_unregfunc(void);
> #define TRACE_DEFINE_ENUM(x)
> #define TRACE_DEFINE_SIZEOF(x)
>
>+#ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
>+static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
>+{
>+ return offset_to_ptr(p);
>+}
>+
>+#define __TRACEPOINT_ENTRY(name) \
>+ asm(" .section \"__tracepoints_ptrs\", \"a\" \n" \
>+ " .balign 4 \n" \
>+ " .long __tracepoint_" #name " - . \n" \
>+ " .previous \n")
>+#else
>+static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
>+{
>+ return *p;
>+}
>+
>+#define __TRACEPOINT_ENTRY(name) \
>+ static tracepoint_ptr_t __tracepoint_ptr_##name __used \
>+ __attribute__((section("__tracepoints_ptrs"))) = \
>+ &__tracepoint_##name
>+#endif
>+
> #endif /* _LINUX_TRACEPOINT_H */
>
> /*
>@@ -253,19 +276,6 @@ extern void syscall_unregfunc(void);
> return static_key_false(&__tracepoint_##name.key); \
> }
>
>-#ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
>-#define __TRACEPOINT_ENTRY(name) \
>- asm(" .section \"__tracepoints_ptrs\", \"a\" \n" \
>- " .balign 4 \n" \
>- " .long __tracepoint_" #name " - . \n" \
>- " .previous \n")
>-#else
>-#define __TRACEPOINT_ENTRY(name) \
>- static struct tracepoint * const __tracepoint_ptr_##name __used \
>- __attribute__((section("__tracepoints_ptrs"))) = \
>- &__tracepoint_##name
>-#endif
>-
> /*
> * We have no guarantee that gcc and the linker won't up-align the tracepoint
> * structures, so we create an array of pointers that will be used for iteration
>diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
>index bf2c06ef9afc..a3be42304485 100644
>--- a/kernel/tracepoint.c
>+++ b/kernel/tracepoint.c
>@@ -28,8 +28,8 @@
> #include <linux/sched/task.h>
> #include <linux/static_key.h>
>
>-extern struct tracepoint * const __start___tracepoints_ptrs[];
>-extern struct tracepoint * const __stop___tracepoints_ptrs[];
>+extern tracepoint_ptr_t __start___tracepoints_ptrs[];
>+extern tracepoint_ptr_t __stop___tracepoints_ptrs[];
>
> DEFINE_SRCU(tracepoint_srcu);
> EXPORT_SYMBOL_GPL(tracepoint_srcu);
>@@ -371,25 +371,17 @@ int tracepoint_probe_unregister(struct tracepoint *tp, void *probe, void *data)
> }
> EXPORT_SYMBOL_GPL(tracepoint_probe_unregister);
>
>-static void for_each_tracepoint_range(struct tracepoint * const *begin,
>- struct tracepoint * const *end,
>+static void for_each_tracepoint_range(
>+ tracepoint_ptr_t *begin, tracepoint_ptr_t *end,
> void (*fct)(struct tracepoint *tp, void *priv),
> void *priv)
> {
>+ tracepoint_ptr_t *iter;
>+
> if (!begin)
> return;
>-
>- if (IS_ENABLED(CONFIG_HAVE_ARCH_PREL32_RELOCATIONS)) {
>- const int *iter;
>-
>- for (iter = (const int *)begin; iter < (const int *)end; iter++)
>- fct(offset_to_ptr(iter), priv);
>- } else {
>- struct tracepoint * const *iter;
>-
>- for (iter = begin; iter < end; iter++)
>- fct(*iter, priv);
>- }
>+ for (iter = begin; iter < end; iter++)
>+ fct(tracepoint_ptr_deref(iter), priv);
> }
>
> #ifdef CONFIG_MODULES
>--
>2.17.1
>

2018-10-15 15:49:36

by Steven Rostedt

[permalink] [raw]

Subject: Re: [PATCH for 4.19] tracepoint: Fix: out-of-bound tracepoint array iteration

On Mon, 15 Oct 2018 13:34:33 +0200
Jessica Yu <[email protected]> wrote:

> For module parts:
>
> Acked-by: Jessica Yu <[email protected]>

Thanks Jessica!

Mathieu, I'm going to pull this into my test queue and start running
tests on it. It should be ready by tomorrow, if none of the tests fail
(note, I'm testing more than one patch, so hopefully none of them fail).

-- Steve