[Cocci] [Fwd: Re: [PATCH] rtc: sun6i: Use struct_size() in kzalloc()]

Kees Cook keescook at chromium.org
Fri Aug 24 00:06:41 CEST 2018


On Thu, Aug 23, 2018 at 3:00 PM, Julia Lawall <julia.lawall at lip6.fr> wrote:
>
>
> On Thu, 23 Aug 2018, Kees Cook wrote:
>
>> On Thu, Aug 23, 2018 at 2:48 PM, Joe Perches <joe at perches.com> wrote:
>> > Forwarding a question about coccinelle and isomorphisms from Kees Cook
>> >
>> > ---------- Forwarded message ----------
>> > From: Kees Cook <keescook at chromium.org>
>> > To: "Gustavo A. R. Silva" <gustavo at embeddedor.com>
>> > Cc: Alessandro Zummo <a.zummo at towertech.it>, Alexandre Belloni <alexandre.belloni at bootlin.com>, Maxime Ripard <maxime.ripard at bootlin.com>, Chen-Yu Tsai <wens at csie.org>, linux-rtc at vger.kernel.org, linux-arm-kernel <linux-arm-kernel at lists.infradead.org>, LKML <linux-kernel at vger.kernel.org>
>> > Bcc:
>> > Date: Thu, 23 Aug 2018 13:56:35 -0700
>> > Subject: Re: [PATCH] rtc: sun6i: Use struct_size() in kzalloc()
>> > On Thu, Aug 23, 2018 at 11:51 AM, Gustavo A. R. Silva
>> > <gustavo at embeddedor.com> wrote:
>> >> One of the more common cases of allocation size calculations is finding
>> >> the size of a structure that has a zero-sized array at the end, along
>> >> with memory for some number of elements for that array. For example:
>> >>
>> >> struct foo {
>> >>         int stuff;
>> >>         void *entry[];
>> >> };
>> >>
>> >> instance = kzalloc(sizeof(struct foo) + sizeof(void *) * count, GFP_KERNEL);
>> >>
>> >> Instead of leaving these open-coded and prone to type mistakes, we can
>> >> now use the new struct_size() helper:
>> >>
>> >> instance = kzalloc(struct_size(instance, entry, count), GFP_KERNEL);
>> >>
>> >> Signed-off-by: Gustavo A. R. Silva <gustavo at embeddedor.com>
>> >> ---
>> >>  drivers/rtc/rtc-sun6i.c | 3 +--
>> >>  1 file changed, 1 insertion(+), 2 deletions(-)
>> >>
>> >> diff --git a/drivers/rtc/rtc-sun6i.c b/drivers/rtc/rtc-sun6i.c
>> >> index 2cd5a7b..fe07310 100644
>> >> --- a/drivers/rtc/rtc-sun6i.c
>> >> +++ b/drivers/rtc/rtc-sun6i.c
>> >> @@ -199,8 +199,7 @@ static void __init sun6i_rtc_clk_init(struct device_node *node)
>> >>         if (!rtc)
>> >>                 return;
>> >>
>> >> -       clk_data = kzalloc(sizeof(*clk_data) + (sizeof(*clk_data->hws) * 2),
>> >> -                          GFP_KERNEL);
>> >> +       clk_data = kzalloc(struct_size(clk_data, hws, 2), GFP_KERNEL);
>> >>         if (!clk_data) {
>> >>                 kfree(rtc);
>> >>                 return;
>> >
>> > This looks like entirely correct to me, but I'm surprised the
>> > Coccinelle script didn't discover this. I guess the isomorphisms don't
>> > cover the parenthesis?
>>
>> I had this:
>>
>> @@
>> identifier alloc =~
>> "kmalloc|kzalloc|kmalloc_node|kzalloc_node|vmalloc|vzalloc|kvmalloc|kvzalloc";
>> identifier VAR, ELEMENT;
>> expression COUNT;
>> @@
>>
>> - alloc(sizeof(*VAR) + COUNT * sizeof(*VAR->ELEMENT)
>> + alloc(struct_size(VAR, ELEMENT, COUNT)
>>   , ...)
>>
>> But I needed to explicitly change the rule to:
>>
>> (
>> - alloc(sizeof(*VAR) + COUNT * sizeof(*VAR->ELEMENT)
>> + alloc(struct_size(VAR, ELEMENT, COUNT)
>>   , ...)
>> |
>> - alloc(sizeof(*VAR) + (COUNT * sizeof(*VAR->ELEMENT))
>> + alloc(struct_size(VAR, ELEMENT, COUNT)
>>   , ...)
>> )
>>
>> to add the ()s. I would expect arithmetic commutative expressions to
>> match? But I had to add parens?
>
> Isomorphisms don't add parens.  They only remove them.  If they added
> them, you would end up with the possibility of having them everywhere, in
> all permutations, which would be slow and useless.

Would a rule for:

a + (b * c)

match:

a + b * c

?

-Kees

-- 
Kees Cook
Pixel Security


More information about the Cocci mailing list