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

Julia Lawall julia.lawall at lip6.fr
Fri Aug 24 00:00:04 CEST 2018



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.

julia


More information about the Cocci mailing list