From: "Morten Brørup" <mb@smartsharesystems.com>
To: "Jerin Jacob" <jerinjacobk@gmail.com>,
"Marat Khalili" <marat.khalili@huawei.com>
Cc: "Konstantin Ananyev" <konstantin.ananyev@huawei.com>,
"Stephen Hemminger" <stephen@networkplumber.org>, <dev@dpdk.org>
Subject: RE: [PATCH 1/3] bpf: fix signed shift overflows in ARM JIT
Date: Tue, 11 Nov 2025 08:53:09 +0100 [thread overview]
Message-ID: <98CBD80474FA8B44BF855DF32C47DC35F65556@smartserver.smartshare.dk> (raw)
In-Reply-To: <CALBAE1O2VtvLV06-4hPCb5qHsRzqDM1iWWOaRcPKu9L-NMu2qg@mail.gmail.com>
> From: Jerin Jacob [mailto:jerinjacobk@gmail.com]
> Sent: Tuesday, 11 November 2025 07.25
>
> On Mon, Nov 10, 2025 at 9:01 PM Marat Khalili
> <marat.khalili@huawei.com> wrote:
> >
> > Left shifts of integer literals and bool values overwriting the sign
> bit
> > were used multiple times in bpf_jit_arm64.c. E.g.:
> >
> > insn = (!!is64) << 31;
> >
> > where is64 has type bool (double bang is a no-op here). The operand
> of
> > left shift was promoted to type int, which when 32-bit wide cannot
> > represent the result. Similarly literal integers have int type by
> > default. Sanitizer produced the following diagnostic during runtime
> > (for various lines):
> >
> > lib/bpf/bpf_jit_arm64.c:241:18: runtime error: left shift of 1 by
> 31
> > places cannot be represented in type 'int'
>
> Wonder why none of the tests in app/test/test_bpf.c able to catch
> this? The generated ARM opcode looks OK (otherwise tests wont pass).
> Could you check what is missing in the app/test/test_bpf.c?
>
> Also SHIFT_VAR32 needs goto common code.
>
>
> >
> > To fix the issue use RTE_BIT32 and similar macros instead.
> >
> > Signed-off-by: Marat Khalili <marat.khalili@huawei.com>
> > ---
> > lib/bpf/bpf_jit_arm64.c | 188 +++++++++++++++++++++++---------------
> --
> > 1 file changed, 107 insertions(+), 81 deletions(-)
> >
> > diff --git a/lib/bpf/bpf_jit_arm64.c b/lib/bpf/bpf_jit_arm64.c
> > index 96b8cd2e03..5f43db0170 100644
> > --- a/lib/bpf/bpf_jit_arm64.c
> > +++ b/lib/bpf/bpf_jit_arm64.c
> > @@ -28,7 +28,33 @@
> > #define A64_ZR 31
> >
> > #define check_imm(n, val) (((val) >= 0) ? !!((val) >> (n)) :
> !!((~val) >> (n)))
> > -#define mask_imm(n, val) ((val) & ((1 << (n)) - 1))
> > +#define mask_imm(n, val) ((val) & (RTE_BIT32(n) - 1))
> > +
> > +/**
> > + * Get the uint32_t shifted value.
> > + *
> > + * Works similarly to RTE_SHIFT_VAL32 but accepts non-literal
> arguments.
> > + * Performs identically to RTE_SHIFT_VAL32 with literal arguments.
> > + *
> > + * @param val
> > + * The value to be shifted, can be non-literal.
> > + * @param nr
> > + * The shift number in range of 0 to (32 - width of val).
> > + */
> > +#define SHIFT_VAR32(val, nr) ((uint32_t)(val) << (nr))
> > +
> > +/**
> > + * Get the uint64_t shifted value.
> > + *
> > + * Works similarly to RTE_SHIFT_VAL64 but accepts non-literal
> arguments.
> > + * Performs identically to RTE_SHIFT_VAL64 with literal arguments.
> > + *
> > + * @param val
> > + * The value to be shifted, can be non-literal.
> > + * @param nr
> > + * The shift number in range of 0 to (64 - width of val).
> > + */
> > +#define SHIFT_VAR64(val, nr) ((uint64_t)(val) << (nr))
Better replace the RTE_SHIFT_VAL32/VAL64 macros with these, to support both literal and non-literal arguments.
next prev parent reply other threads:[~2025-11-11 7:53 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-11-10 15:30 [PATCH 0/3] bpf: simple tests and fixes Marat Khalili
2025-11-10 15:30 ` [PATCH 1/3] bpf: fix signed shift overflows in ARM JIT Marat Khalili
2025-11-11 6:25 ` Jerin Jacob
2025-11-11 7:53 ` Morten Brørup [this message]
2025-11-11 10:10 ` Marat Khalili
2025-11-11 16:29 ` Jerin Jacob
2025-11-11 16:31 ` Jerin Jacob
2025-11-11 16:39 ` Marat Khalili
2025-11-12 5:23 ` Jerin Jacob
2025-11-10 15:30 ` [PATCH 2/3] bpf: disallow empty program Marat Khalili
2025-11-10 16:40 ` Stephen Hemminger
2025-11-10 16:46 ` Marat Khalili
2025-11-10 15:30 ` [PATCH 3/3] bpf: make add/subtract one program validate Marat Khalili
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=98CBD80474FA8B44BF855DF32C47DC35F65556@smartserver.smartshare.dk \
--to=mb@smartsharesystems.com \
--cc=dev@dpdk.org \
--cc=jerinjacobk@gmail.com \
--cc=konstantin.ananyev@huawei.com \
--cc=marat.khalili@huawei.com \
--cc=stephen@networkplumber.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).