* [PATCH v4] x86/insn, tools/x86: Fix some potential undefined behavior.
@ 2021-09-23 16:18 Ian Rogers
2021-09-24 10:45 ` [tip: x86/urgent] x86/insn, tools/x86: Fix undefined behavior due to potential unaligned accesses tip-bot2 for Numfor Mbiziwo-Tiapo
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Ian Rogers @ 2021-09-23 16:18 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
H. Peter Anvin, Peter Zijlstra (Intel),
Masami Hiramatsu, Josh Poimboeuf, linux-kernel, Adrian Hunter,
Arnaldo Carvalho de Melo, David Laight
Cc: Numfor Mbiziwo-Tiapo, Ian Rogers
From: Numfor Mbiziwo-Tiapo <nums@google.com>
Don't perform unaligned loads in __get_next and __peek_nbyte_next as
these are forms of undefined behavior.
These problems were identified using the undefined behavior sanitizer
(ubsan) with the tools version of the code and perf test. Part of this
patch was previously posted here:
https://lore.kernel.org/lkml/20190724184512.162887-4-nums@google.com/
v4. Fixes a typo.
v3. Is a rebase picking up a fix for big endian architectures.
v2. removes the validate_next check and merges the 2 changes into one as
requested by Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Ian Rogers <irogers@google.com>
Signed-off-by: Numfor Mbiziwo-Tiapo <nums@google.com>
Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
---
arch/x86/lib/insn.c | 4 ++--
tools/arch/x86/lib/insn.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/x86/lib/insn.c b/arch/x86/lib/insn.c
index 058f19b20465..c565def611e2 100644
--- a/arch/x86/lib/insn.c
+++ b/arch/x86/lib/insn.c
@@ -37,10 +37,10 @@
((insn)->next_byte + sizeof(t) + n <= (insn)->end_kaddr)
#define __get_next(t, insn) \
- ({ t r = *(t*)insn->next_byte; insn->next_byte += sizeof(t); leXX_to_cpu(t, r); })
+ ({ t r; memcpy(&r, insn->next_byte, sizeof(t)); insn->next_byte += sizeof(t); leXX_to_cpu(t, r); })
#define __peek_nbyte_next(t, insn, n) \
- ({ t r = *(t*)((insn)->next_byte + n); leXX_to_cpu(t, r); })
+ ({ t r; memcpy(&r, (insn)->next_byte + n, sizeof(t)); leXX_to_cpu(t, r); })
#define get_next(t, insn) \
({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
diff --git a/tools/arch/x86/lib/insn.c b/tools/arch/x86/lib/insn.c
index c41f95815480..797699462cd8 100644
--- a/tools/arch/x86/lib/insn.c
+++ b/tools/arch/x86/lib/insn.c
@@ -37,10 +37,10 @@
((insn)->next_byte + sizeof(t) + n <= (insn)->end_kaddr)
#define __get_next(t, insn) \
- ({ t r = *(t*)insn->next_byte; insn->next_byte += sizeof(t); leXX_to_cpu(t, r); })
+ ({ t r; memcpy(&r, insn->next_byte, sizeof(t)); insn->next_byte += sizeof(t); leXX_to_cpu(t, r); })
#define __peek_nbyte_next(t, insn, n) \
- ({ t r = *(t*)((insn)->next_byte + n); leXX_to_cpu(t, r); })
+ ({ t r; memcpy(&r, (insn)->next_byte + n, sizeof(t)); leXX_to_cpu(t, r); })
#define get_next(t, insn) \
({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
--
2.33.0.464.g1972c5931b-goog
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [tip: x86/urgent] x86/insn, tools/x86: Fix undefined behavior due to potential unaligned accesses
2021-09-23 16:18 [PATCH v4] x86/insn, tools/x86: Fix some potential undefined behavior Ian Rogers
@ 2021-09-24 10:45 ` tip-bot2 for Numfor Mbiziwo-Tiapo
2021-09-24 19:02 ` [PATCH v4] x86/insn, tools/x86: Fix some potential undefined behavior Arnaldo Carvalho de Melo
2021-09-27 7:36 ` H. Peter Anvin
2 siblings, 0 replies; 8+ messages in thread
From: tip-bot2 for Numfor Mbiziwo-Tiapo @ 2021-09-24 10:45 UTC (permalink / raw)
To: linux-tip-commits
Cc: Numfor Mbiziwo-Tiapo, Ian Rogers, Borislav Petkov,
Masami Hiramatsu, x86, linux-kernel
The following commit has been merged into the x86/urgent branch of tip:
Commit-ID: 5ba1071f7554c4027bdbd712a146111de57918de
Gitweb: https://git.kernel.org/tip/5ba1071f7554c4027bdbd712a146111de57918de
Author: Numfor Mbiziwo-Tiapo <nums@google.com>
AuthorDate: Thu, 23 Sep 2021 09:18:43 -07:00
Committer: Borislav Petkov <bp@suse.de>
CommitterDate: Fri, 24 Sep 2021 12:37:38 +02:00
x86/insn, tools/x86: Fix undefined behavior due to potential unaligned accesses
Don't perform unaligned loads in __get_next() and __peek_nbyte_next() as
these are forms of undefined behavior:
"A pointer to an object or incomplete type may be converted to a pointer
to a different object or incomplete type. If the resulting pointer
is not correctly aligned for the pointed-to type, the behavior is
undefined."
(from http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf)
These problems were identified using the undefined behavior sanitizer
(ubsan) with the tools version of the code and perf test.
[ bp: Massage commit message. ]
Signed-off-by: Numfor Mbiziwo-Tiapo <nums@google.com>
Signed-off-by: Ian Rogers <irogers@google.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
Link: https://lkml.kernel.org/r/20210923161843.751834-1-irogers@google.com
---
arch/x86/lib/insn.c | 4 ++--
tools/arch/x86/lib/insn.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/x86/lib/insn.c b/arch/x86/lib/insn.c
index 058f19b..c565def 100644
--- a/arch/x86/lib/insn.c
+++ b/arch/x86/lib/insn.c
@@ -37,10 +37,10 @@
((insn)->next_byte + sizeof(t) + n <= (insn)->end_kaddr)
#define __get_next(t, insn) \
- ({ t r = *(t*)insn->next_byte; insn->next_byte += sizeof(t); leXX_to_cpu(t, r); })
+ ({ t r; memcpy(&r, insn->next_byte, sizeof(t)); insn->next_byte += sizeof(t); leXX_to_cpu(t, r); })
#define __peek_nbyte_next(t, insn, n) \
- ({ t r = *(t*)((insn)->next_byte + n); leXX_to_cpu(t, r); })
+ ({ t r; memcpy(&r, (insn)->next_byte + n, sizeof(t)); leXX_to_cpu(t, r); })
#define get_next(t, insn) \
({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
diff --git a/tools/arch/x86/lib/insn.c b/tools/arch/x86/lib/insn.c
index c41f958..7976994 100644
--- a/tools/arch/x86/lib/insn.c
+++ b/tools/arch/x86/lib/insn.c
@@ -37,10 +37,10 @@
((insn)->next_byte + sizeof(t) + n <= (insn)->end_kaddr)
#define __get_next(t, insn) \
- ({ t r = *(t*)insn->next_byte; insn->next_byte += sizeof(t); leXX_to_cpu(t, r); })
+ ({ t r; memcpy(&r, insn->next_byte, sizeof(t)); insn->next_byte += sizeof(t); leXX_to_cpu(t, r); })
#define __peek_nbyte_next(t, insn, n) \
- ({ t r = *(t*)((insn)->next_byte + n); leXX_to_cpu(t, r); })
+ ({ t r; memcpy(&r, (insn)->next_byte + n, sizeof(t)); leXX_to_cpu(t, r); })
#define get_next(t, insn) \
({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v4] x86/insn, tools/x86: Fix some potential undefined behavior.
2021-09-23 16:18 [PATCH v4] x86/insn, tools/x86: Fix some potential undefined behavior Ian Rogers
2021-09-24 10:45 ` [tip: x86/urgent] x86/insn, tools/x86: Fix undefined behavior due to potential unaligned accesses tip-bot2 for Numfor Mbiziwo-Tiapo
@ 2021-09-24 19:02 ` Arnaldo Carvalho de Melo
2021-09-25 4:39 ` Masami Hiramatsu
2021-09-27 7:36 ` H. Peter Anvin
2 siblings, 1 reply; 8+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-09-24 19:02 UTC (permalink / raw)
To: Ian Rogers
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
H. Peter Anvin, Peter Zijlstra (Intel),
Masami Hiramatsu, Josh Poimboeuf, linux-kernel, Adrian Hunter,
David Laight, Numfor Mbiziwo-Tiapo
Em Thu, Sep 23, 2021 at 09:18:43AM -0700, Ian Rogers escreveu:
> From: Numfor Mbiziwo-Tiapo <nums@google.com>
>
> Don't perform unaligned loads in __get_next and __peek_nbyte_next as
> these are forms of undefined behavior.
>
> These problems were identified using the undefined behavior sanitizer
> (ubsan) with the tools version of the code and perf test. Part of this
> patch was previously posted here:
> https://lore.kernel.org/lkml/20190724184512.162887-4-nums@google.com/
Masami, if you're ok, just process it including the tools/ bit.
- Arnaldo
> v4. Fixes a typo.
>
> v3. Is a rebase picking up a fix for big endian architectures.
>
> v2. removes the validate_next check and merges the 2 changes into one as
> requested by Masami Hiramatsu <mhiramat@kernel.org>
>
> Signed-off-by: Ian Rogers <irogers@google.com>
> Signed-off-by: Numfor Mbiziwo-Tiapo <nums@google.com>
> Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
> ---
> arch/x86/lib/insn.c | 4 ++--
> tools/arch/x86/lib/insn.c | 4 ++--
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/lib/insn.c b/arch/x86/lib/insn.c
> index 058f19b20465..c565def611e2 100644
> --- a/arch/x86/lib/insn.c
> +++ b/arch/x86/lib/insn.c
> @@ -37,10 +37,10 @@
> ((insn)->next_byte + sizeof(t) + n <= (insn)->end_kaddr)
>
> #define __get_next(t, insn) \
> - ({ t r = *(t*)insn->next_byte; insn->next_byte += sizeof(t); leXX_to_cpu(t, r); })
> + ({ t r; memcpy(&r, insn->next_byte, sizeof(t)); insn->next_byte += sizeof(t); leXX_to_cpu(t, r); })
>
> #define __peek_nbyte_next(t, insn, n) \
> - ({ t r = *(t*)((insn)->next_byte + n); leXX_to_cpu(t, r); })
> + ({ t r; memcpy(&r, (insn)->next_byte + n, sizeof(t)); leXX_to_cpu(t, r); })
>
> #define get_next(t, insn) \
> ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
> diff --git a/tools/arch/x86/lib/insn.c b/tools/arch/x86/lib/insn.c
> index c41f95815480..797699462cd8 100644
> --- a/tools/arch/x86/lib/insn.c
> +++ b/tools/arch/x86/lib/insn.c
> @@ -37,10 +37,10 @@
> ((insn)->next_byte + sizeof(t) + n <= (insn)->end_kaddr)
>
> #define __get_next(t, insn) \
> - ({ t r = *(t*)insn->next_byte; insn->next_byte += sizeof(t); leXX_to_cpu(t, r); })
> + ({ t r; memcpy(&r, insn->next_byte, sizeof(t)); insn->next_byte += sizeof(t); leXX_to_cpu(t, r); })
>
> #define __peek_nbyte_next(t, insn, n) \
> - ({ t r = *(t*)((insn)->next_byte + n); leXX_to_cpu(t, r); })
> + ({ t r; memcpy(&r, (insn)->next_byte + n, sizeof(t)); leXX_to_cpu(t, r); })
>
> #define get_next(t, insn) \
> ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
> --
> 2.33.0.464.g1972c5931b-goog
--
- Arnaldo
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v4] x86/insn, tools/x86: Fix some potential undefined behavior.
2021-09-24 19:02 ` [PATCH v4] x86/insn, tools/x86: Fix some potential undefined behavior Arnaldo Carvalho de Melo
@ 2021-09-25 4:39 ` Masami Hiramatsu
2021-09-26 11:59 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 8+ messages in thread
From: Masami Hiramatsu @ 2021-09-25 4:39 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Ian Rogers, Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
H. Peter Anvin, Peter Zijlstra (Intel),
Masami Hiramatsu, Josh Poimboeuf, linux-kernel, Adrian Hunter,
David Laight, Numfor Mbiziwo-Tiapo
On Fri, 24 Sep 2021 16:02:33 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> Em Thu, Sep 23, 2021 at 09:18:43AM -0700, Ian Rogers escreveu:
> > From: Numfor Mbiziwo-Tiapo <nums@google.com>
> >
> > Don't perform unaligned loads in __get_next and __peek_nbyte_next as
> > these are forms of undefined behavior.
> >
> > These problems were identified using the undefined behavior sanitizer
> > (ubsan) with the tools version of the code and perf test. Part of this
> > patch was previously posted here:
> > https://lore.kernel.org/lkml/20190724184512.162887-4-nums@google.com/
>
> Masami, if you're ok, just process it including the tools/ bit.
Hi Arnaldo,
This version updates the tools/ too, so I think this is OK.
(do I need re-Ack?)
Thank you,
>
> - Arnaldo
>
> > v4. Fixes a typo.
> >
> > v3. Is a rebase picking up a fix for big endian architectures.
> >
> > v2. removes the validate_next check and merges the 2 changes into one as
> > requested by Masami Hiramatsu <mhiramat@kernel.org>
> >
> > Signed-off-by: Ian Rogers <irogers@google.com>
> > Signed-off-by: Numfor Mbiziwo-Tiapo <nums@google.com>
> > Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
> > ---
> > arch/x86/lib/insn.c | 4 ++--
> > tools/arch/x86/lib/insn.c | 4 ++--
> > 2 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/x86/lib/insn.c b/arch/x86/lib/insn.c
> > index 058f19b20465..c565def611e2 100644
> > --- a/arch/x86/lib/insn.c
> > +++ b/arch/x86/lib/insn.c
> > @@ -37,10 +37,10 @@
> > ((insn)->next_byte + sizeof(t) + n <= (insn)->end_kaddr)
> >
> > #define __get_next(t, insn) \
> > - ({ t r = *(t*)insn->next_byte; insn->next_byte += sizeof(t); leXX_to_cpu(t, r); })
> > + ({ t r; memcpy(&r, insn->next_byte, sizeof(t)); insn->next_byte += sizeof(t); leXX_to_cpu(t, r); })
> >
> > #define __peek_nbyte_next(t, insn, n) \
> > - ({ t r = *(t*)((insn)->next_byte + n); leXX_to_cpu(t, r); })
> > + ({ t r; memcpy(&r, (insn)->next_byte + n, sizeof(t)); leXX_to_cpu(t, r); })
> >
> > #define get_next(t, insn) \
> > ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
> > diff --git a/tools/arch/x86/lib/insn.c b/tools/arch/x86/lib/insn.c
> > index c41f95815480..797699462cd8 100644
> > --- a/tools/arch/x86/lib/insn.c
> > +++ b/tools/arch/x86/lib/insn.c
> > @@ -37,10 +37,10 @@
> > ((insn)->next_byte + sizeof(t) + n <= (insn)->end_kaddr)
> >
> > #define __get_next(t, insn) \
> > - ({ t r = *(t*)insn->next_byte; insn->next_byte += sizeof(t); leXX_to_cpu(t, r); })
> > + ({ t r; memcpy(&r, insn->next_byte, sizeof(t)); insn->next_byte += sizeof(t); leXX_to_cpu(t, r); })
> >
> > #define __peek_nbyte_next(t, insn, n) \
> > - ({ t r = *(t*)((insn)->next_byte + n); leXX_to_cpu(t, r); })
> > + ({ t r; memcpy(&r, (insn)->next_byte + n, sizeof(t)); leXX_to_cpu(t, r); })
> >
> > #define get_next(t, insn) \
> > ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
> > --
> > 2.33.0.464.g1972c5931b-goog
>
> --
>
> - Arnaldo
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v4] x86/insn, tools/x86: Fix some potential undefined behavior.
2021-09-25 4:39 ` Masami Hiramatsu
@ 2021-09-26 11:59 ` Arnaldo Carvalho de Melo
2021-09-26 15:25 ` Borislav Petkov
0 siblings, 1 reply; 8+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-09-26 11:59 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Ian Rogers, Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
H. Peter Anvin, Peter Zijlstra (Intel),
Josh Poimboeuf, linux-kernel, Adrian Hunter, David Laight,
Numfor Mbiziwo-Tiapo
Em Sat, Sep 25, 2021 at 01:39:44PM +0900, Masami Hiramatsu escreveu:
> On Fri, 24 Sep 2021 16:02:33 -0300
> Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
> > Em Thu, Sep 23, 2021 at 09:18:43AM -0700, Ian Rogers escreveu:
> > > From: Numfor Mbiziwo-Tiapo <nums@google.com>
> > >
> > > Don't perform unaligned loads in __get_next and __peek_nbyte_next as
> > > these are forms of undefined behavior.
> > >
> > > These problems were identified using the undefined behavior sanitizer
> > > (ubsan) with the tools version of the code and perf test. Part of this
> > > patch was previously posted here:
> > > https://lore.kernel.org/lkml/20190724184512.162887-4-nums@google.com/
> >
> > Masami, if you're ok, just process it including the tools/ bit.
>
> Hi Arnaldo,
>
> This version updates the tools/ too, so I think this is OK.
> (do I need re-Ack?)
So you want me to process it?
- Arnaldo
> Thank you,
>
> >
> > - Arnaldo
> >
> > > v4. Fixes a typo.
> > >
> > > v3. Is a rebase picking up a fix for big endian architectures.
> > >
> > > v2. removes the validate_next check and merges the 2 changes into one as
> > > requested by Masami Hiramatsu <mhiramat@kernel.org>
> > >
> > > Signed-off-by: Ian Rogers <irogers@google.com>
> > > Signed-off-by: Numfor Mbiziwo-Tiapo <nums@google.com>
> > > Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
> > > ---
> > > arch/x86/lib/insn.c | 4 ++--
> > > tools/arch/x86/lib/insn.c | 4 ++--
> > > 2 files changed, 4 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/arch/x86/lib/insn.c b/arch/x86/lib/insn.c
> > > index 058f19b20465..c565def611e2 100644
> > > --- a/arch/x86/lib/insn.c
> > > +++ b/arch/x86/lib/insn.c
> > > @@ -37,10 +37,10 @@
> > > ((insn)->next_byte + sizeof(t) + n <= (insn)->end_kaddr)
> > >
> > > #define __get_next(t, insn) \
> > > - ({ t r = *(t*)insn->next_byte; insn->next_byte += sizeof(t); leXX_to_cpu(t, r); })
> > > + ({ t r; memcpy(&r, insn->next_byte, sizeof(t)); insn->next_byte += sizeof(t); leXX_to_cpu(t, r); })
> > >
> > > #define __peek_nbyte_next(t, insn, n) \
> > > - ({ t r = *(t*)((insn)->next_byte + n); leXX_to_cpu(t, r); })
> > > + ({ t r; memcpy(&r, (insn)->next_byte + n, sizeof(t)); leXX_to_cpu(t, r); })
> > >
> > > #define get_next(t, insn) \
> > > ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
> > > diff --git a/tools/arch/x86/lib/insn.c b/tools/arch/x86/lib/insn.c
> > > index c41f95815480..797699462cd8 100644
> > > --- a/tools/arch/x86/lib/insn.c
> > > +++ b/tools/arch/x86/lib/insn.c
> > > @@ -37,10 +37,10 @@
> > > ((insn)->next_byte + sizeof(t) + n <= (insn)->end_kaddr)
> > >
> > > #define __get_next(t, insn) \
> > > - ({ t r = *(t*)insn->next_byte; insn->next_byte += sizeof(t); leXX_to_cpu(t, r); })
> > > + ({ t r; memcpy(&r, insn->next_byte, sizeof(t)); insn->next_byte += sizeof(t); leXX_to_cpu(t, r); })
> > >
> > > #define __peek_nbyte_next(t, insn, n) \
> > > - ({ t r = *(t*)((insn)->next_byte + n); leXX_to_cpu(t, r); })
> > > + ({ t r; memcpy(&r, (insn)->next_byte + n, sizeof(t)); leXX_to_cpu(t, r); })
> > >
> > > #define get_next(t, insn) \
> > > ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
> > > --
> > > 2.33.0.464.g1972c5931b-goog
> >
> > --
> >
> > - Arnaldo
>
>
> --
> Masami Hiramatsu <mhiramat@kernel.org>
--
- Arnaldo
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v4] x86/insn, tools/x86: Fix some potential undefined behavior.
2021-09-26 11:59 ` Arnaldo Carvalho de Melo
@ 2021-09-26 15:25 ` Borislav Petkov
2021-09-27 12:33 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 8+ messages in thread
From: Borislav Petkov @ 2021-09-26 15:25 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Masami Hiramatsu, Ian Rogers, Thomas Gleixner, Ingo Molnar, x86,
H. Peter Anvin, Peter Zijlstra (Intel),
Josh Poimboeuf, linux-kernel, Adrian Hunter, David Laight,
Numfor Mbiziwo-Tiapo
On Sun, Sep 26, 2021 at 08:59:35AM -0300, Arnaldo Carvalho de Melo wrote:
> So you want me to process it?
https://git.kernel.org/tip/5ba1071f7554c4027bdbd712a146111de57918de
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v4] x86/insn, tools/x86: Fix some potential undefined behavior.
2021-09-23 16:18 [PATCH v4] x86/insn, tools/x86: Fix some potential undefined behavior Ian Rogers
2021-09-24 10:45 ` [tip: x86/urgent] x86/insn, tools/x86: Fix undefined behavior due to potential unaligned accesses tip-bot2 for Numfor Mbiziwo-Tiapo
2021-09-24 19:02 ` [PATCH v4] x86/insn, tools/x86: Fix some potential undefined behavior Arnaldo Carvalho de Melo
@ 2021-09-27 7:36 ` H. Peter Anvin
2 siblings, 0 replies; 8+ messages in thread
From: H. Peter Anvin @ 2021-09-27 7:36 UTC (permalink / raw)
To: Ian Rogers, Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
Peter Zijlstra (Intel),
Masami Hiramatsu, Josh Poimboeuf, linux-kernel, Adrian Hunter,
Arnaldo Carvalho de Melo, David Laight
Cc: Numfor Mbiziwo-Tiapo
We have get_unaligned() for a reason.
On September 23, 2021 9:18:43 AM PDT, Ian Rogers <irogers@google.com> wrote:
>From: Numfor Mbiziwo-Tiapo <nums@google.com>
>
>Don't perform unaligned loads in __get_next and __peek_nbyte_next as
>these are forms of undefined behavior.
>
>These problems were identified using the undefined behavior sanitizer
>(ubsan) with the tools version of the code and perf test. Part of this
>patch was previously posted here:
>https://lore.kernel.org/lkml/20190724184512.162887-4-nums@google.com/
>
>v4. Fixes a typo.
>
>v3. Is a rebase picking up a fix for big endian architectures.
>
>v2. removes the validate_next check and merges the 2 changes into one as
>requested by Masami Hiramatsu <mhiramat@kernel.org>
>
>Signed-off-by: Ian Rogers <irogers@google.com>
>Signed-off-by: Numfor Mbiziwo-Tiapo <nums@google.com>
>Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
>---
> arch/x86/lib/insn.c | 4 ++--
> tools/arch/x86/lib/insn.c | 4 ++--
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
>diff --git a/arch/x86/lib/insn.c b/arch/x86/lib/insn.c
>index 058f19b20465..c565def611e2 100644
>--- a/arch/x86/lib/insn.c
>+++ b/arch/x86/lib/insn.c
>@@ -37,10 +37,10 @@
> ((insn)->next_byte + sizeof(t) + n <= (insn)->end_kaddr)
>
> #define __get_next(t, insn) \
>- ({ t r = *(t*)insn->next_byte; insn->next_byte += sizeof(t); leXX_to_cpu(t, r); })
>+ ({ t r; memcpy(&r, insn->next_byte, sizeof(t)); insn->next_byte += sizeof(t); leXX_to_cpu(t, r); })
>
> #define __peek_nbyte_next(t, insn, n) \
>- ({ t r = *(t*)((insn)->next_byte + n); leXX_to_cpu(t, r); })
>+ ({ t r; memcpy(&r, (insn)->next_byte + n, sizeof(t)); leXX_to_cpu(t, r); })
>
> #define get_next(t, insn) \
> ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
>diff --git a/tools/arch/x86/lib/insn.c b/tools/arch/x86/lib/insn.c
>index c41f95815480..797699462cd8 100644
>--- a/tools/arch/x86/lib/insn.c
>+++ b/tools/arch/x86/lib/insn.c
>@@ -37,10 +37,10 @@
> ((insn)->next_byte + sizeof(t) + n <= (insn)->end_kaddr)
>
> #define __get_next(t, insn) \
>- ({ t r = *(t*)insn->next_byte; insn->next_byte += sizeof(t); leXX_to_cpu(t, r); })
>+ ({ t r; memcpy(&r, insn->next_byte, sizeof(t)); insn->next_byte += sizeof(t); leXX_to_cpu(t, r); })
>
> #define __peek_nbyte_next(t, insn, n) \
>- ({ t r = *(t*)((insn)->next_byte + n); leXX_to_cpu(t, r); })
>+ ({ t r; memcpy(&r, (insn)->next_byte + n, sizeof(t)); leXX_to_cpu(t, r); })
>
> #define get_next(t, insn) \
> ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v4] x86/insn, tools/x86: Fix some potential undefined behavior.
2021-09-26 15:25 ` Borislav Petkov
@ 2021-09-27 12:33 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 8+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-09-27 12:33 UTC (permalink / raw)
To: Borislav Petkov
Cc: Masami Hiramatsu, Ian Rogers, Thomas Gleixner, Ingo Molnar, x86,
H. Peter Anvin, Peter Zijlstra (Intel),
Josh Poimboeuf, linux-kernel, Adrian Hunter, David Laight,
Numfor Mbiziwo-Tiapo
Em Sun, Sep 26, 2021 at 05:25:44PM +0200, Borislav Petkov escreveu:
> On Sun, Sep 26, 2021 at 08:59:35AM -0300, Arnaldo Carvalho de Melo wrote:
> > So you want me to process it?
>
> https://git.kernel.org/tip/5ba1071f7554c4027bdbd712a146111de57918de
Ok, processed already, case closed :-)
- Arnaldo
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-09-27 12:33 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-23 16:18 [PATCH v4] x86/insn, tools/x86: Fix some potential undefined behavior Ian Rogers
2021-09-24 10:45 ` [tip: x86/urgent] x86/insn, tools/x86: Fix undefined behavior due to potential unaligned accesses tip-bot2 for Numfor Mbiziwo-Tiapo
2021-09-24 19:02 ` [PATCH v4] x86/insn, tools/x86: Fix some potential undefined behavior Arnaldo Carvalho de Melo
2021-09-25 4:39 ` Masami Hiramatsu
2021-09-26 11:59 ` Arnaldo Carvalho de Melo
2021-09-26 15:25 ` Borislav Petkov
2021-09-27 12:33 ` Arnaldo Carvalho de Melo
2021-09-27 7:36 ` H. Peter Anvin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).