| Message ID | 6662597c-13a2-5c6e-df6c-31d18ed34bfd@rub.de (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | fix u32 printf specifier | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On Wed, Jun 02, 2021 at 05:23:19PM +0200, Benedict Schlueter wrote: > Hi, > > I assume its clear what this patch does. > > > From 9618e4475b812651c3fe481af885757675fc4ae2 Mon Sep 17 00:00:00 2001 > From: Benedict Schlueter <benedict.schlueter@rub.de> > Date: Wed, 2 Jun 2021 17:16:13 +0200 > Subject: use correct format string specifier for unsigned 32 Bit > bounds print statements > > Signed-off-by: Benedict Schlueter <benedict.schlueter@rub.de> > --- > kernel/bpf/verifier.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > index 1de4b8c6ee42..e107996c7220 100644 > --- a/kernel/bpf/verifier.c > +++ b/kernel/bpf/verifier.c > @@ -690,11 +690,11 @@ static void print_verifier_state(struct > bpf_verifier_env *env, > (int)(reg->s32_max_value)); > if (reg->u32_min_value != reg->umin_value && > reg->u32_min_value != U32_MIN) > - verbose(env, ",u32_min_value=%d", > + verbose(env, ",u32_min_value=%u", > (int)(reg->u32_min_value)); "%u" and (int) cast don't make sense. It needs a proper commit message to explain why the change is needed and also a Fixes tag. Please refer to Documentation/bpf/bpf_devel_QA.rst.
On 02/06/2021 19:41, Martin KaFai Lau wrote: > On Wed, Jun 02, 2021 at 05:23:19PM +0200, Benedict Schlueter wrote: >> Hi, >> >> I assume its clear what this patch does. >> >> >> From 9618e4475b812651c3fe481af885757675fc4ae2 Mon Sep 17 00:00:00 2001 >> From: Benedict Schlueter <benedict.schlueter@rub.de> >> Date: Wed, 2 Jun 2021 17:16:13 +0200 >> Subject: use correct format string specifier for unsigned 32 Bit >> bounds print statements >> >> Signed-off-by: Benedict Schlueter <benedict.schlueter@rub.de> >> --- >> kernel/bpf/verifier.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c >> index 1de4b8c6ee42..e107996c7220 100644 >> --- a/kernel/bpf/verifier.c >> +++ b/kernel/bpf/verifier.c >> @@ -690,11 +690,11 @@ static void print_verifier_state(struct >> bpf_verifier_env *env, >> (int)(reg->s32_max_value)); >> if (reg->u32_min_value != reg->umin_value && >> reg->u32_min_value != U32_MIN) >> - verbose(env, ",u32_min_value=%d", >> + verbose(env, ",u32_min_value=%u", >> (int)(reg->u32_min_value)); > "%u" and (int) cast don't make sense. Yep, changed to unsigned int for consistency with the other cases. Is this necessary? Since reg->u32_min_value is already a unsigned 32 bit number. > It needs a proper commit message to explain why the change is needed > and also a Fixes tag. Please refer to Documentation/bpf/bpf_devel_QA.rst. Sorry should have read this more carefully before. Everything should be included right now. From fd076dc5f2bd5ec4e9cb49530e77cf2d3e4f42c2 Mon Sep 17 00:00:00 2001 From: Benedict Schlueter <benedict.schlueter@rub.de> Date: Wed, 2 Jun 2021 21:42:39 +0200 Subject: [PATCH bpf-next] use correct format string specifier for unsigned 32 bounds when printing an unsigned value, it should be a positive number verifier log before the patch ([...],s32_max_value=-2,u32_min_value=-16,u32_max_value=-2) verifier log after the patch ([...],s32_max_value=-2,u32_min_value=4294967280,u32_max_value=4294967294) fixes 3f50f132d840 (bpf: Verifier, do explicit ALU32 bounds tracking) Signed-off-by: Benedict Schlueter <benedict.schlueter@rub.de> --- kernel/bpf/verifier.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 1de4b8c6ee42..ea482ebaeb26 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -690,12 +690,12 @@ static void print_verifier_state(struct bpf_verifier_env *env, (int)(reg->s32_max_value)); if (reg->u32_min_value != reg->umin_value && reg->u32_min_value != U32_MIN) - verbose(env, ",u32_min_value=%d", - (int)(reg->u32_min_value)); + verbose(env, ",u32_min_value=%u", + (unsigned int)(reg->u32_min_value)); if (reg->u32_max_value != reg->umax_value && reg->u32_max_value != U32_MAX) - verbose(env, ",u32_max_value=%d", - (int)(reg->u32_max_value)); + verbose(env, ",u32_max_value=%u", + (unsigned int)(reg->u32_max_value)); } verbose(env, ")"); }
On Wed, Jun 2, 2021 at 3:43 PM Benedict Schlueter <Benedict.Schlueter@rub.de> wrote: > > On 02/06/2021 19:41, Martin KaFai Lau wrote: > > > On Wed, Jun 02, 2021 at 05:23:19PM +0200, Benedict Schlueter wrote: > >> Hi, > >> > >> I assume its clear what this patch does. > >> > >> > >> From 9618e4475b812651c3fe481af885757675fc4ae2 Mon Sep 17 00:00:00 2001 > >> From: Benedict Schlueter <benedict.schlueter@rub.de> > >> Date: Wed, 2 Jun 2021 17:16:13 +0200 > >> Subject: use correct format string specifier for unsigned 32 Bit > >> bounds print statements > >> > >> Signed-off-by: Benedict Schlueter <benedict.schlueter@rub.de> > >> --- > >> kernel/bpf/verifier.c | 4 ++-- > >> 1 file changed, 2 insertions(+), 2 deletions(-) > >> > >> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > >> index 1de4b8c6ee42..e107996c7220 100644 > >> --- a/kernel/bpf/verifier.c > >> +++ b/kernel/bpf/verifier.c > >> @@ -690,11 +690,11 @@ static void print_verifier_state(struct > >> bpf_verifier_env *env, > >> (int)(reg->s32_max_value)); > >> if (reg->u32_min_value != reg->umin_value && > >> reg->u32_min_value != U32_MIN) > >> - verbose(env, ",u32_min_value=%d", > >> + verbose(env, ",u32_min_value=%u", > >> (int)(reg->u32_min_value)); > > "%u" and (int) cast don't make sense. > Yep, changed to unsigned int for consistency with the other cases. Is > this necessary? Since reg->u32_min_value is already a unsigned 32 bit > number. > > It needs a proper commit message to explain why the change is needed > > and also a Fixes tag. Please refer to Documentation/bpf/bpf_devel_QA.rst. > > Sorry should have read this more carefully before. Everything should be > included right now. > > From fd076dc5f2bd5ec4e9cb49530e77cf2d3e4f42c2 Mon Sep 17 00:00:00 2001 > From: Benedict Schlueter <benedict.schlueter@rub.de> > Date: Wed, 2 Jun 2021 21:42:39 +0200 > Subject: [PATCH bpf-next] > use correct format string specifier for unsigned 32 bounds Please resubmit the patch. See 'Submitting patches' in Documentation/bpf/bpf_devel_QA.rst
On Thu, Jun 03, 2021 at 12:41:57AM +0200, Benedict Schlueter wrote: > On 02/06/2021 19:41, Martin KaFai Lau wrote: > > > On Wed, Jun 02, 2021 at 05:23:19PM +0200, Benedict Schlueter wrote: > > > Hi, > > > > > > I assume its clear what this patch does. > > > > > > > > > From 9618e4475b812651c3fe481af885757675fc4ae2 Mon Sep 17 00:00:00 2001 > > > From: Benedict Schlueter <benedict.schlueter@rub.de> > > > Date: Wed, 2 Jun 2021 17:16:13 +0200 > > > Subject: use correct format string specifier for unsigned 32 Bit > > > bounds print statements > > > > > > Signed-off-by: Benedict Schlueter <benedict.schlueter@rub.de> > > > --- > > > kernel/bpf/verifier.c | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > > > index 1de4b8c6ee42..e107996c7220 100644 > > > --- a/kernel/bpf/verifier.c > > > +++ b/kernel/bpf/verifier.c > > > @@ -690,11 +690,11 @@ static void print_verifier_state(struct > > > bpf_verifier_env *env, > > > (int)(reg->s32_max_value)); > > > if (reg->u32_min_value != reg->umin_value && > > > reg->u32_min_value != U32_MIN) > > > - verbose(env, ",u32_min_value=%d", > > > + verbose(env, ",u32_min_value=%u", > > > (int)(reg->u32_min_value)); > > "%u" and (int) cast don't make sense. > Yep, changed to unsigned int for consistency with the other cases. Is this > necessary? Since reg->u32_min_value is already a unsigned 32 bit number. cast is unnecessary. > > It needs a proper commit message to explain why the change is needed > > and also a Fixes tag. Please refer to Documentation/bpf/bpf_devel_QA.rst. > > Sorry should have read this more carefully before. Everything should be > included right now. > > From fd076dc5f2bd5ec4e9cb49530e77cf2d3e4f42c2 Mon Sep 17 00:00:00 2001 > From: Benedict Schlueter <benedict.schlueter@rub.de> > Date: Wed, 2 Jun 2021 21:42:39 +0200 > Subject: [PATCH bpf-next] > use correct format string specifier for unsigned 32 bounds > > when printing an unsigned value, it should be a positive number > > verifier log before the patch > ([...],s32_max_value=-2,u32_min_value=-16,u32_max_value=-2) > > verifier log after the patch > ([...],s32_max_value=-2,u32_min_value=4294967280,u32_max_value=4294967294) > > > fixes 3f50f132d840 (bpf: Verifier, do explicit ALU32 bounds tracking) Fixes: 3f50f132d840 ("bpf: Verifier, do explicit ALU32 bounds tracking") which can be obtained by: git log -1 --format='Fixes: %h ("%s")' 3f50f132d840
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 1de4b8c6ee42..e107996c7220 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -690,11 +690,11 @@ static void print_verifier_state(struct bpf_verifier_env *env, (int)(reg->s32_max_value)); if (reg->u32_min_value != reg->umin_value && reg->u32_min_value != U32_MIN) - verbose(env, ",u32_min_value=%d", + verbose(env, ",u32_min_value=%u", (int)(reg->u32_min_value)); if (reg->u32_max_value != reg->umax_value && reg->u32_max_value != U32_MAX) - verbose(env, ",u32_max_value=%d", + verbose(env, ",u32_max_value=%u", (int)(reg->u32_max_value)); }