DPDK patches and discussions
 help / color / mirror / Atom feed
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.


  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).