linux-kernel - Re: [PATCH] bpf, btf: Make if test explicit to fix Coccinelle error

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <6d0a07082a064fa8b410a7e08d8b6b628845ac72.camel@gmail.com>
Date: Mon, 24 Jun 2024 16:27:27 -0700
From: Eduard Zingerman <eddyz87@...il.com>
To: Thorsten Blum <thorsten.blum@...lux.com>
Cc: martin.lau@...ux.dev, ast@...nel.org, daniel@...earbox.net,
 andrii@...nel.org,  song@...nel.org, john.fastabend@...il.com,
 kpsingh@...nel.org, sdf@...ichev.me,  haoluo@...gle.com, jolsa@...nel.org,
 yonghong.song@...ux.dev, bpf@...r.kernel.org,  linux-kernel@...r.kernel.org
Subject: Re: [PATCH] bpf, btf: Make if test explicit to fix Coccinelle error

On Mon, 2024-06-24 at 16:08 -0700, Thorsten Blum wrote:
> On 24. Jun 2024, at 13:16, Eduard Zingerman <eddyz87@...il.com> wrote:
> > On Mon, 2024-06-24 at 21:54 +0200, Thorsten Blum wrote:
> > > Explicitly test the iterator variable i > 0 to fix the following
> > > Coccinelle/coccicheck error reported by itnull.cocci:
> > > 
> > > ERROR: iterator variable bound on line 4688 cannot be NULL

[...]

> > #define for_each_vsi(i, datasec_type, member) \
> > for (i = 0, member = btf_type_var_secinfo(datasec_type); \
> >     i < btf_type_vlen(datasec_type); \
> >     i++, member++)
> > 
> > Here it sets 'i' to zero for the first iteration.
> > Why would the tool report that 'i' can't be zero?
> 
> Coccinelle thinks i can't be a NULL pointer (not the number zero). It's
> essentially a false-positive warning, but since there are only 4 such
> warnings under kernel/, I thought it would be worthwhile to remove some
> of them by making the tests explicit.

Sorry, not really familiar with the tool, but it seems like the
following part of the itnull.cocci fires the warning:

  @r depends on !patch exists@
  iterator I;
  expression x,E;
  position p1,p2;
  @@
  
  *I@p1(x,...)
  { ... when != x = E
  (
  *  x@p2 == NULL
  |
  *  x@p2 != NULL
  )
    ... when any
  }
  
  @script:python depends on org@
  p1 << r.p1;
  p2 << r.p2;
  @@
  
  cocci.print_main("iterator-bound variable",p1)
  cocci.print_secs("useless NULL test",p2)
  
  @script:python depends on report@
  p1 << r.p1;
  p2 << r.p2;
  @@
  
  msg = "ERROR: iterator variable bound on line %s cannot be NULL" % (p1[0].line)
  coccilib.report.print_report(p2[0], msg)

Is there a way to add a constraint here, requiring 'x' to have a pointer type?
(So that the rule does not match, as it clearly shouldn't).