* [PATCH] comedi: Annotate struct comedi_lrange with __counted_by
@ 2023-09-30 9:14 Christophe JAILLET
2023-09-30 20:57 ` Kees Cook
2023-10-01 6:43 ` Gustavo A. R. Silva
0 siblings, 2 replies; 14+ messages in thread
From: Christophe JAILLET @ 2023-09-30 9:14 UTC (permalink / raw)
To: Ian Abbott, H Hartley Sweeten, Kees Cook, Gustavo A. R. Silva,
Nathan Chancellor, Nick Desaulniers, Tom Rix
Cc: linux-kernel, kernel-janitors, Christophe JAILLET,
linux-hardening, llvm
Prepare for the coming implementation by GCC and Clang of the __counted_by
attribute. Flexible array members annotated with __counted_by can have
their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS
(for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family
functions).
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
This patch is part of a work done in parallel of what is currently worked
on by Kees Cook.
My patches are only related to corner cases that do NOT match the
semantic of his Coccinelle script[1].
In this case, it is been spotted because of comedi_alloc_spriv().
All other usages of struct comedi_lrange seem to be static definition of
the structure that explicitly set the .length field.
[1] https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci
---
include/linux/comedi/comedidev.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/comedi/comedidev.h b/include/linux/comedi/comedidev.h
index 0a1150900ef3..c08416a7364b 100644
--- a/include/linux/comedi/comedidev.h
+++ b/include/linux/comedi/comedidev.h
@@ -633,7 +633,7 @@ extern const struct comedi_lrange range_unknown;
*/
struct comedi_lrange {
int length;
- struct comedi_krange range[];
+ struct comedi_krange range[] __counted_by(length);
};
/**
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] comedi: Annotate struct comedi_lrange with __counted_by
2023-09-30 9:14 [PATCH] comedi: Annotate struct comedi_lrange with __counted_by Christophe JAILLET
@ 2023-09-30 20:57 ` Kees Cook
2023-10-01 7:25 ` Julia Lawall
2023-10-01 7:45 ` Julia Lawall
2023-10-01 6:43 ` Gustavo A. R. Silva
1 sibling, 2 replies; 14+ messages in thread
From: Kees Cook @ 2023-09-30 20:57 UTC (permalink / raw)
To: Christophe JAILLET
Cc: Ian Abbott, H Hartley Sweeten, Gustavo A. R. Silva,
Nathan Chancellor, Nick Desaulniers, Tom Rix, linux-kernel,
kernel-janitors, linux-hardening, llvm
On Sat, Sep 30, 2023 at 11:14:47AM +0200, Christophe JAILLET wrote:
> Prepare for the coming implementation by GCC and Clang of the __counted_by
> attribute. Flexible array members annotated with __counted_by can have
> their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS
> (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family
> functions).
>
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> This patch is part of a work done in parallel of what is currently worked
> on by Kees Cook.
>
> My patches are only related to corner cases that do NOT match the
> semantic of his Coccinelle script[1].
Nice!
struct comedi_lrange {
int length;
struct comedi_krange range[];
};
...
static const struct comedi_lrange range_rti800_ai_10_bipolar = {
4, {
BIP_RANGE(10),
BIP_RANGE(1),
BIP_RANGE(0.1),
BIP_RANGE(0.02)
}
};
I'm struggling to come up with a way for Coccinelle to find this kind of
thing in other places...
> In this case, it is been spotted because of comedi_alloc_spriv().
> All other usages of struct comedi_lrange seem to be static definition of
> the structure that explicitly set the .length field.
Ah-ha, I found it in drivers/comedi/drivers/das16.c das16_ai_range():
lrange = comedi_alloc_spriv(s,
struct_size(lrange, range, 1));
I was also able to find this:
union jr3_pci_single_range {
struct comedi_lrange l;
char _reserved[offsetof(struct comedi_lrange, range[1])];
};
Which looks a lot like DEFINE_FLEX:
https://lore.kernel.org/linux-hardening/20230912115937.1645707-2-przemyslaw.kitszel@intel.com/
But that above for stack varaibles rather than globals. But I'm way off
topic now. ;)
Reviewed-by: Kees Cook <keescook@chromium.org>
>
> [1] https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci
> ---
> include/linux/comedi/comedidev.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/comedi/comedidev.h b/include/linux/comedi/comedidev.h
> index 0a1150900ef3..c08416a7364b 100644
> --- a/include/linux/comedi/comedidev.h
> +++ b/include/linux/comedi/comedidev.h
> @@ -633,7 +633,7 @@ extern const struct comedi_lrange range_unknown;
> */
> struct comedi_lrange {
> int length;
> - struct comedi_krange range[];
> + struct comedi_krange range[] __counted_by(length);
> };
>
> /**
> --
> 2.34.1
>
--
Kees Cook
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] comedi: Annotate struct comedi_lrange with __counted_by
2023-09-30 9:14 [PATCH] comedi: Annotate struct comedi_lrange with __counted_by Christophe JAILLET
2023-09-30 20:57 ` Kees Cook
@ 2023-10-01 6:43 ` Gustavo A. R. Silva
1 sibling, 0 replies; 14+ messages in thread
From: Gustavo A. R. Silva @ 2023-10-01 6:43 UTC (permalink / raw)
To: Christophe JAILLET, Ian Abbott, H Hartley Sweeten, Kees Cook,
Gustavo A. R. Silva, Nathan Chancellor, Nick Desaulniers, Tom Rix
Cc: linux-kernel, kernel-janitors, linux-hardening, llvm
On 9/30/23 11:14, Christophe JAILLET wrote:
> Prepare for the coming implementation by GCC and Clang of the __counted_by
> attribute. Flexible array members annotated with __counted_by can have
> their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS
> (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family
> functions).
>
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Thanks
--
Gustavo
> ---
> This patch is part of a work done in parallel of what is currently worked
> on by Kees Cook.
>
> My patches are only related to corner cases that do NOT match the
> semantic of his Coccinelle script[1].
>
> In this case, it is been spotted because of comedi_alloc_spriv().
> All other usages of struct comedi_lrange seem to be static definition of
> the structure that explicitly set the .length field.
>
> [1] https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci
> ---
> include/linux/comedi/comedidev.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/comedi/comedidev.h b/include/linux/comedi/comedidev.h
> index 0a1150900ef3..c08416a7364b 100644
> --- a/include/linux/comedi/comedidev.h
> +++ b/include/linux/comedi/comedidev.h
> @@ -633,7 +633,7 @@ extern const struct comedi_lrange range_unknown;
> */
> struct comedi_lrange {
> int length;
> - struct comedi_krange range[];
> + struct comedi_krange range[] __counted_by(length);
> };
>
> /**
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] comedi: Annotate struct comedi_lrange with __counted_by
2023-09-30 20:57 ` Kees Cook
@ 2023-10-01 7:25 ` Julia Lawall
2023-10-01 7:50 ` Christophe JAILLET
2023-10-01 7:45 ` Julia Lawall
1 sibling, 1 reply; 14+ messages in thread
From: Julia Lawall @ 2023-10-01 7:25 UTC (permalink / raw)
To: Kees Cook
Cc: Christophe JAILLET, Ian Abbott, H Hartley Sweeten,
Gustavo A. R. Silva, Nathan Chancellor, Nick Desaulniers, Tom Rix,
linux-kernel, kernel-janitors, linux-hardening, llvm
On Sat, 30 Sep 2023, Kees Cook wrote:
> On Sat, Sep 30, 2023 at 11:14:47AM +0200, Christophe JAILLET wrote:
> > Prepare for the coming implementation by GCC and Clang of the __counted_by
> > attribute. Flexible array members annotated with __counted_by can have
> > their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS
> > (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family
> > functions).
> >
> > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> > ---
> > This patch is part of a work done in parallel of what is currently worked
> > on by Kees Cook.
> >
> > My patches are only related to corner cases that do NOT match the
> > semantic of his Coccinelle script[1].
>
> Nice!
>
> struct comedi_lrange {
> int length;
> struct comedi_krange range[];
> };
> ...
> static const struct comedi_lrange range_rti800_ai_10_bipolar = {
> 4, {
> BIP_RANGE(10),
> BIP_RANGE(1),
> BIP_RANGE(0.1),
> BIP_RANGE(0.02)
> }
> };
>
> I'm struggling to come up with a way for Coccinelle to find this kind of
> thing in other places...
>
> > In this case, it is been spotted because of comedi_alloc_spriv().
> > All other usages of struct comedi_lrange seem to be static definition of
> > the structure that explicitly set the .length field.
>
> Ah-ha, I found it in drivers/comedi/drivers/das16.c das16_ai_range():
>
> lrange = comedi_alloc_spriv(s,
> struct_size(lrange, range, 1));
This is not found due to the regular expression used for the name of the
alloc function. Maybe you could drop it entirely? Maybe you could just
check for alloc somewhere in the string?
identifier ALLOC =~ "alloc";
works in this case.
Also, I see in the link that you have:
// Options: --all-includes
You can actually force this by putting
#spatch --all-includes
and any other options you want.
julia
>
> I was also able to find this:
>
> union jr3_pci_single_range {
> struct comedi_lrange l;
> char _reserved[offsetof(struct comedi_lrange, range[1])];
> };
>
> Which looks a lot like DEFINE_FLEX:
> https://lore.kernel.org/linux-hardening/20230912115937.1645707-2-przemyslaw.kitszel@intel.com/
> But that above for stack varaibles rather than globals. But I'm way off
> topic now. ;)
>
> Reviewed-by: Kees Cook <keescook@chromium.org>
>
> >
> > [1] https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci
> > ---
> > include/linux/comedi/comedidev.h | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/include/linux/comedi/comedidev.h b/include/linux/comedi/comedidev.h
> > index 0a1150900ef3..c08416a7364b 100644
> > --- a/include/linux/comedi/comedidev.h
> > +++ b/include/linux/comedi/comedidev.h
> > @@ -633,7 +633,7 @@ extern const struct comedi_lrange range_unknown;
> > */
> > struct comedi_lrange {
> > int length;
> > - struct comedi_krange range[];
> > + struct comedi_krange range[] __counted_by(length);
> > };
> >
> > /**
> > --
> > 2.34.1
> >
>
> --
> Kees Cook
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] comedi: Annotate struct comedi_lrange with __counted_by
2023-09-30 20:57 ` Kees Cook
2023-10-01 7:25 ` Julia Lawall
@ 2023-10-01 7:45 ` Julia Lawall
2023-10-01 15:26 ` Kees Cook
1 sibling, 1 reply; 14+ messages in thread
From: Julia Lawall @ 2023-10-01 7:45 UTC (permalink / raw)
To: Kees Cook
Cc: Christophe JAILLET, Ian Abbott, H Hartley Sweeten,
Gustavo A. R. Silva, Nathan Chancellor, Nick Desaulniers, Tom Rix,
linux-kernel, kernel-janitors, linux-hardening, llvm
On Sat, 30 Sep 2023, Kees Cook wrote:
> On Sat, Sep 30, 2023 at 11:14:47AM +0200, Christophe JAILLET wrote:
> > Prepare for the coming implementation by GCC and Clang of the __counted_by
> > attribute. Flexible array members annotated with __counted_by can have
> > their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS
> > (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family
> > functions).
> >
> > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> > ---
> > This patch is part of a work done in parallel of what is currently worked
> > on by Kees Cook.
> >
> > My patches are only related to corner cases that do NOT match the
> > semantic of his Coccinelle script[1].
>
> Nice!
>
> struct comedi_lrange {
> int length;
> struct comedi_krange range[];
> };
> ...
> static const struct comedi_lrange range_rti800_ai_10_bipolar = {
> 4, {
> BIP_RANGE(10),
> BIP_RANGE(1),
> BIP_RANGE(0.1),
> BIP_RANGE(0.02)
> }
> };
>
> I'm struggling to come up with a way for Coccinelle to find this kind of
> thing in other places...
Kees, what exactly are you trying to match? Static allocations?
julia
>
> > In this case, it is been spotted because of comedi_alloc_spriv().
> > All other usages of struct comedi_lrange seem to be static definition of
> > the structure that explicitly set the .length field.
>
> Ah-ha, I found it in drivers/comedi/drivers/das16.c das16_ai_range():
>
> lrange = comedi_alloc_spriv(s,
> struct_size(lrange, range, 1));
>
> I was also able to find this:
>
> union jr3_pci_single_range {
> struct comedi_lrange l;
> char _reserved[offsetof(struct comedi_lrange, range[1])];
> };
>
> Which looks a lot like DEFINE_FLEX:
> https://lore.kernel.org/linux-hardening/20230912115937.1645707-2-przemyslaw.kitszel@intel.com/
> But that above for stack varaibles rather than globals. But I'm way off
> topic now. ;)
>
> Reviewed-by: Kees Cook <keescook@chromium.org>
>
> >
> > [1] https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci
> > ---
> > include/linux/comedi/comedidev.h | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/include/linux/comedi/comedidev.h b/include/linux/comedi/comedidev.h
> > index 0a1150900ef3..c08416a7364b 100644
> > --- a/include/linux/comedi/comedidev.h
> > +++ b/include/linux/comedi/comedidev.h
> > @@ -633,7 +633,7 @@ extern const struct comedi_lrange range_unknown;
> > */
> > struct comedi_lrange {
> > int length;
> > - struct comedi_krange range[];
> > + struct comedi_krange range[] __counted_by(length);
> > };
> >
> > /**
> > --
> > 2.34.1
> >
>
> --
> Kees Cook
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] comedi: Annotate struct comedi_lrange with __counted_by
2023-10-01 7:25 ` Julia Lawall
@ 2023-10-01 7:50 ` Christophe JAILLET
0 siblings, 0 replies; 14+ messages in thread
From: Christophe JAILLET @ 2023-10-01 7:50 UTC (permalink / raw)
To: Julia Lawall, Kees Cook
Cc: Ian Abbott, H Hartley Sweeten, Gustavo A. R. Silva,
Nathan Chancellor, Nick Desaulniers, Tom Rix, linux-kernel,
kernel-janitors, linux-hardening, llvm
Le 01/10/2023 à 09:25, Julia Lawall a écrit :
>
> This is not found due to the regular expression used for the name of the
> alloc function. Maybe you could drop it entirely? Maybe you could just
> check for alloc somewhere in the string?
That's how I found it.
I simplified a lot Kees's script and looked for function names that did
*not* match his regex.
Functions that:
- return a pointer to a struct
- are used with struct_size()
- store the value used to compite the size in another field of the
struct
are good enough candidates.
I think that removing the regex all together would be just good enough.
CJ
>
> identifier ALLOC =~ "alloc";
>
> works in this case.
>
> Also, I see in the link that you have:
>
> // Options: --all-includes
>
> You can actually force this by putting
>
> #spatch --all-includes
Nice, thanks for the tip.
>
> and any other options you want.
>
> julia
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] comedi: Annotate struct comedi_lrange with __counted_by
2023-10-01 7:45 ` Julia Lawall
@ 2023-10-01 15:26 ` Kees Cook
2023-10-01 19:14 ` Julia Lawall
0 siblings, 1 reply; 14+ messages in thread
From: Kees Cook @ 2023-10-01 15:26 UTC (permalink / raw)
To: Julia Lawall, Kees Cook
Cc: Christophe JAILLET, Ian Abbott, H Hartley Sweeten,
Gustavo A. R. Silva, Nathan Chancellor, Nick Desaulniers, Tom Rix,
linux-kernel, kernel-janitors, linux-hardening, llvm
On October 1, 2023 12:45:41 AM PDT, Julia Lawall <julia.lawall@inria.fr> wrote:
>
>
>On Sat, 30 Sep 2023, Kees Cook wrote:
>
>> On Sat, Sep 30, 2023 at 11:14:47AM +0200, Christophe JAILLET wrote:
>> > Prepare for the coming implementation by GCC and Clang of the __counted_by
>> > attribute. Flexible array members annotated with __counted_by can have
>> > their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS
>> > (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family
>> > functions).
>> >
>> > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
>> > ---
>> > This patch is part of a work done in parallel of what is currently worked
>> > on by Kees Cook.
>> >
>> > My patches are only related to corner cases that do NOT match the
>> > semantic of his Coccinelle script[1].
>>
>> Nice!
>>
>> struct comedi_lrange {
>> int length;
>> struct comedi_krange range[];
>> };
>> ...
>> static const struct comedi_lrange range_rti800_ai_10_bipolar = {
>> 4, {
>> BIP_RANGE(10),
>> BIP_RANGE(1),
>> BIP_RANGE(0.1),
>> BIP_RANGE(0.02)
>> }
>> };
>>
>> I'm struggling to come up with a way for Coccinelle to find this kind of
>> thing in other places...
>
>Kees, what exactly are you trying to match? Static allocations?
I need to count the number of initialized elements in the flexible array that is the last member and see if it matches a value set in another member.
E.g. the above sets 4 values for the last array member and then sets another member to 4.
-Kees
--
Kees Cook
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] comedi: Annotate struct comedi_lrange with __counted_by
2023-10-01 15:26 ` Kees Cook
@ 2023-10-01 19:14 ` Julia Lawall
2023-10-01 21:05 ` Kees Cook
0 siblings, 1 reply; 14+ messages in thread
From: Julia Lawall @ 2023-10-01 19:14 UTC (permalink / raw)
To: Kees Cook
Cc: Julia Lawall, Kees Cook, Christophe JAILLET, Ian Abbott,
H Hartley Sweeten, Gustavo A. R. Silva, Nathan Chancellor,
Nick Desaulniers, Tom Rix, linux-kernel, kernel-janitors,
linux-hardening, llvm
Kees,
You can try the following.
julia
#spatch --all-includes
@r@
identifier i,j;
type T;
@@
struct i {
...
T j[];
}
@s@
identifier r.i;
constant ini;
identifier j;
initializer list [n] is2;
position p;
identifier x;
@@
struct i@p x =
{ ...,
.j = ini,
...,
{ is2 } }
;
@script:ocaml@
ini << s.ini;
i << r.i;
j << s.j;
n << s.n;
p << s.p;
@@
try
let ini = int_of_string ini in
if n = ini
then Printf.printf "%s:%d: struct %s: field %s is the counter for the flex array\n" (List.hd p).file (List.hd p).line i j
with _-> () (* not an explicit integer *)
@s2@
identifier r.i;
constant ini;
initializer list [n] is;
initializer list [n2] is2;
position p;
identifier x;
@@
struct i@p x =
{ is,
ini,
...,
{ is2 } };
@script:ocaml@
ini << s2.ini;
i << r.i;
n << s2.n;
n2 << s2.n2;
p << s2.p;
@@
try
let ini = int_of_string ini in
if n2 = ini
then Printf.printf "%s:%d: struct %s: field at offset %d is the counter for the flex array\n" (List.hd p).file (List.hd p).line i n
with _-> () (* not an explicit integer *)
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] comedi: Annotate struct comedi_lrange with __counted_by
2023-10-01 19:14 ` Julia Lawall
@ 2023-10-01 21:05 ` Kees Cook
2023-10-01 21:22 ` Kees Cook
0 siblings, 1 reply; 14+ messages in thread
From: Kees Cook @ 2023-10-01 21:05 UTC (permalink / raw)
To: Julia Lawall
Cc: Kees Cook, Christophe JAILLET, Ian Abbott, H Hartley Sweeten,
Gustavo A. R. Silva, Nathan Chancellor, Nick Desaulniers, Tom Rix,
linux-kernel, kernel-janitors, linux-hardening, llvm
On Sun, Oct 01, 2023 at 09:14:02PM +0200, Julia Lawall wrote:
> Kees,
>
> You can try the following.
Cool! Yeah, this finds the example:
drivers/comedi/drivers/rti800.c:74: struct comedi_lrange: field at offset 0 is the counter for the flex array
drivers/comedi/drivers/rti800.c:83: struct comedi_lrange: field at offset 0 is the counter for the flex array
drivers/comedi/drivers/rti800.c:92: struct comedi_lrange: field at offset 0 is the counter for the flex array
I'll run it on the whole codebase...
--
Kees Cook
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] comedi: Annotate struct comedi_lrange with __counted_by
2023-10-01 21:05 ` Kees Cook
@ 2023-10-01 21:22 ` Kees Cook
2023-10-01 22:21 ` Kees Cook
2023-11-05 21:27 ` Christophe JAILLET
0 siblings, 2 replies; 14+ messages in thread
From: Kees Cook @ 2023-10-01 21:22 UTC (permalink / raw)
To: Julia Lawall
Cc: Kees Cook, Christophe JAILLET, Ian Abbott, H Hartley Sweeten,
Gustavo A. R. Silva, Nathan Chancellor, Nick Desaulniers, Tom Rix,
linux-kernel, kernel-janitors, linux-hardening, llvm
On Sun, Oct 01, 2023 at 02:05:46PM -0700, Kees Cook wrote:
> On Sun, Oct 01, 2023 at 09:14:02PM +0200, Julia Lawall wrote:
> > Kees,
> >
> > You can try the following.
>
> Cool! Yeah, this finds the example:
>
> drivers/comedi/drivers/rti800.c:74: struct comedi_lrange: field at offset 0 is the counter for the flex array
> drivers/comedi/drivers/rti800.c:83: struct comedi_lrange: field at offset 0 is the counter for the flex array
> drivers/comedi/drivers/rti800.c:92: struct comedi_lrange: field at offset 0 is the counter for the flex array
>
> I'll run it on the whole codebase...
It found only the struct comedi_lrange instances, but that's good to
know. :)
--
Kees Cook
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] comedi: Annotate struct comedi_lrange with __counted_by
2023-10-01 21:22 ` Kees Cook
@ 2023-10-01 22:21 ` Kees Cook
2023-10-02 5:38 ` Julia Lawall
2023-11-05 21:27 ` Christophe JAILLET
1 sibling, 1 reply; 14+ messages in thread
From: Kees Cook @ 2023-10-01 22:21 UTC (permalink / raw)
To: Julia Lawall
Cc: Kees Cook, Christophe JAILLET, Ian Abbott, H Hartley Sweeten,
Gustavo A. R. Silva, Nathan Chancellor, Nick Desaulniers, Tom Rix,
linux-kernel, kernel-janitors, linux-hardening, llvm
On Sun, Oct 01, 2023 at 02:22:17PM -0700, Kees Cook wrote:
> On Sun, Oct 01, 2023 at 02:05:46PM -0700, Kees Cook wrote:
> > On Sun, Oct 01, 2023 at 09:14:02PM +0200, Julia Lawall wrote:
> > > Kees,
> > >
> > > You can try the following.
> >
> > Cool! Yeah, this finds the example:
> >
> > drivers/comedi/drivers/rti800.c:74: struct comedi_lrange: field at offset 0 is the counter for the flex array
> > drivers/comedi/drivers/rti800.c:83: struct comedi_lrange: field at offset 0 is the counter for the flex array
> > drivers/comedi/drivers/rti800.c:92: struct comedi_lrange: field at offset 0 is the counter for the flex array
> >
> > I'll run it on the whole codebase...
>
> It found only the struct comedi_lrange instances, but that's good to
> know. :)
On a related note, why doesn't this work?
@allocated@
identifier STRUCT, ARRAY;
expression COUNT;
struct STRUCT *PTR;
identifier ALLOC;
type ELEMENT_TYPE;
@@
PTR = ALLOC(..., sizeof(\(*PTR\|struct STRUCT\)) +
COUNT * sizeof(\(*PTR->ARRAY\|PTR->ARRAY[0]\|ELEMENT_TYPE\)), ...);
minus: parse error:
File "alloc.cocci", line 15, column 34, charpos = 485
around = 'struct',
whole content = PTR = ALLOC(..., sizeof(\(*PTR\|struct STRUCT\)) +
if I drop "struct", then it complains about ELEMENT_TYPE...
-Kees
--
Kees Cook
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] comedi: Annotate struct comedi_lrange with __counted_by
2023-10-01 22:21 ` Kees Cook
@ 2023-10-02 5:38 ` Julia Lawall
2023-10-02 7:36 ` Kees Cook
0 siblings, 1 reply; 14+ messages in thread
From: Julia Lawall @ 2023-10-02 5:38 UTC (permalink / raw)
To: Kees Cook
Cc: Julia Lawall, Kees Cook, Christophe JAILLET, Ian Abbott,
H Hartley Sweeten, Gustavo A. R. Silva, Nathan Chancellor,
Nick Desaulniers, Tom Rix, linux-kernel, kernel-janitors,
linux-hardening, llvm
On Sun, 1 Oct 2023, Kees Cook wrote:
> On Sun, Oct 01, 2023 at 02:22:17PM -0700, Kees Cook wrote:
> > On Sun, Oct 01, 2023 at 02:05:46PM -0700, Kees Cook wrote:
> > > On Sun, Oct 01, 2023 at 09:14:02PM +0200, Julia Lawall wrote:
> > > > Kees,
> > > >
> > > > You can try the following.
> > >
> > > Cool! Yeah, this finds the example:
> > >
> > > drivers/comedi/drivers/rti800.c:74: struct comedi_lrange: field at offset 0 is the counter for the flex array
> > > drivers/comedi/drivers/rti800.c:83: struct comedi_lrange: field at offset 0 is the counter for the flex array
> > > drivers/comedi/drivers/rti800.c:92: struct comedi_lrange: field at offset 0 is the counter for the flex array
> > >
> > > I'll run it on the whole codebase...
> >
> > It found only the struct comedi_lrange instances, but that's good to
> > know. :)
>
> On a related note, why doesn't this work?
>
> @allocated@
> identifier STRUCT, ARRAY;
> expression COUNT;
> struct STRUCT *PTR;
> identifier ALLOC;
> type ELEMENT_TYPE;
> @@
>
> PTR = ALLOC(..., sizeof(\(*PTR\|struct STRUCT\)) +
> COUNT * sizeof(\(*PTR->ARRAY\|PTR->ARRAY[0]\|ELEMENT_TYPE\)), ...);
>
>
> minus: parse error:
> File "alloc.cocci", line 15, column 34, charpos = 485
> around = 'struct',
> whole content = PTR = ALLOC(..., sizeof(\(*PTR\|struct STRUCT\)) +
>
> if I drop "struct", then it complains about ELEMENT_TYPE...
The sizeof with an expression argument is treated differently than the
sizeof with a type argument. So you need to write:
@allocated@
identifier STRUCT, ARRAY;
expression COUNT;
struct STRUCT *PTR;
identifier ALLOC;
type ELEMENT_TYPE;
@@
PTR = ALLOC(..., \(sizeof(*PTR)\|sizeof(struct STRUCT)\) +
COUNT * \(sizeof(*PTR->ARRAY)\|sizeof(PTR->ARRAY[0])\|sizeof(ELEMENT_TYPE)\), ...);
julia
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] comedi: Annotate struct comedi_lrange with __counted_by
2023-10-02 5:38 ` Julia Lawall
@ 2023-10-02 7:36 ` Kees Cook
0 siblings, 0 replies; 14+ messages in thread
From: Kees Cook @ 2023-10-02 7:36 UTC (permalink / raw)
To: Julia Lawall
Cc: Kees Cook, Christophe JAILLET, Ian Abbott, H Hartley Sweeten,
Gustavo A. R. Silva, Nathan Chancellor, Nick Desaulniers, Tom Rix,
linux-kernel, kernel-janitors, linux-hardening, llvm
On Mon, Oct 02, 2023 at 07:38:42AM +0200, Julia Lawall wrote:
> The sizeof with an expression argument is treated differently than the
> sizeof with a type argument. So you need to write:
>
> @allocated@
> identifier STRUCT, ARRAY;
> expression COUNT;
> struct STRUCT *PTR;
> identifier ALLOC;
> type ELEMENT_TYPE;
> @@
>
> PTR = ALLOC(..., \(sizeof(*PTR)\|sizeof(struct STRUCT)\) +
> COUNT * \(sizeof(*PTR->ARRAY)\|sizeof(PTR->ARRAY[0])\|sizeof(ELEMENT_TYPE)\), ...);
Ah! Thank you thank you! Yes, this works great now. :)
--
Kees Cook
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] comedi: Annotate struct comedi_lrange with __counted_by
2023-10-01 21:22 ` Kees Cook
2023-10-01 22:21 ` Kees Cook
@ 2023-11-05 21:27 ` Christophe JAILLET
1 sibling, 0 replies; 14+ messages in thread
From: Christophe JAILLET @ 2023-11-05 21:27 UTC (permalink / raw)
To: Kees Cook, Julia Lawall
Cc: Kees Cook, Gustavo A. R. Silva, linux-kernel, kernel-janitors,
linux-hardening
Le 01/10/2023 à 23:22, Kees Cook a écrit :
> On Sun, Oct 01, 2023 at 02:05:46PM -0700, Kees Cook wrote:
>> On Sun, Oct 01, 2023 at 09:14:02PM +0200, Julia Lawall wrote:
>>> Kees,
>>>
>>> You can try the following.
>>
>> Cool! Yeah, this finds the example:
>>
>> drivers/comedi/drivers/rti800.c:74: struct comedi_lrange: field at offset 0 is the counter for the flex array
>> drivers/comedi/drivers/rti800.c:83: struct comedi_lrange: field at offset 0 is the counter for the flex array
>> drivers/comedi/drivers/rti800.c:92: struct comedi_lrange: field at offset 0 is the counter for the flex array
>>
>> I'll run it on the whole codebase...
>
> It found only the struct comedi_lrange instances, but that's good to
> know. :)
>
Hi,
(removing most of people from the thread)
I found another one.
struct ocotp_devtype_data has a field 'num_entry' which is the number of
entries in 'entry' flex array.
[1]:
https://elixir.bootlin.com/linux/v6.6/source/drivers/nvmem/imx-ocotp-ele.c#L28
[2]:
https://elixir.bootlin.com/linux/v6.6/source/drivers/nvmem/imx-ocotp-ele.c#L143
CJ
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2023-11-05 21:28 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-30 9:14 [PATCH] comedi: Annotate struct comedi_lrange with __counted_by Christophe JAILLET
2023-09-30 20:57 ` Kees Cook
2023-10-01 7:25 ` Julia Lawall
2023-10-01 7:50 ` Christophe JAILLET
2023-10-01 7:45 ` Julia Lawall
2023-10-01 15:26 ` Kees Cook
2023-10-01 19:14 ` Julia Lawall
2023-10-01 21:05 ` Kees Cook
2023-10-01 21:22 ` Kees Cook
2023-10-01 22:21 ` Kees Cook
2023-10-02 5:38 ` Julia Lawall
2023-10-02 7:36 ` Kees Cook
2023-11-05 21:27 ` Christophe JAILLET
2023-10-01 6:43 ` Gustavo A. R. Silva
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox