* [PATCH] rtc: sun6i: Use struct_size() in kzalloc()
@ 2018-08-23 18:51 ` Gustavo A. R. Silva
0 siblings, 0 replies; 20+ messages in thread
From: Gustavo A. R. Silva @ 2018-08-23 18:51 UTC (permalink / raw)
To: Alessandro Zummo, Alexandre Belloni, Maxime Ripard, Chen-Yu Tsai
Cc: linux-rtc, linux-arm-kernel, linux-kernel, Kees Cook,
Gustavo A. R. Silva
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@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;
--
2.7.4
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH] rtc: sun6i: Use struct_size() in kzalloc() @ 2018-08-23 18:51 ` Gustavo A. R. Silva 0 siblings, 0 replies; 20+ messages in thread From: Gustavo A. R. Silva @ 2018-08-23 18:51 UTC (permalink / raw) To: linux-arm-kernel 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@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; -- 2.7.4 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH] rtc: sun6i: Use struct_size() in kzalloc() 2018-08-23 18:51 ` Gustavo A. R. Silva @ 2018-08-23 20:56 ` Kees Cook -1 siblings, 0 replies; 20+ messages in thread From: Kees Cook @ 2018-08-23 20:56 UTC (permalink / raw) To: Gustavo A. R. Silva Cc: Alessandro Zummo, Alexandre Belloni, Maxime Ripard, Chen-Yu Tsai, linux-rtc, linux-arm-kernel, LKML On Thu, Aug 23, 2018 at 11:51 AM, Gustavo A. R. Silva <gustavo@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@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? -Kees -- Kees Cook Pixel Security ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH] rtc: sun6i: Use struct_size() in kzalloc() @ 2018-08-23 20:56 ` Kees Cook 0 siblings, 0 replies; 20+ messages in thread From: Kees Cook @ 2018-08-23 20:56 UTC (permalink / raw) To: linux-arm-kernel On Thu, Aug 23, 2018 at 11:51 AM, Gustavo A. R. Silva <gustavo@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@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? -Kees -- Kees Cook Pixel Security ^ permalink raw reply [flat|nested] 20+ messages in thread
[parent not found: <700bfdf296aa112799b6a6f091162cc34f6fef10.camel@perches.com>]
* [Cocci] [Fwd: Re: [PATCH] rtc: sun6i: Use struct_size() in kzalloc()] [not found] ` <700bfdf296aa112799b6a6f091162cc34f6fef10.camel@perches.com> @ 2018-08-23 21:56 ` Kees Cook 2018-08-23 22:00 ` Julia Lawall 0 siblings, 1 reply; 20+ messages in thread From: Kees Cook @ 2018-08-23 21:56 UTC (permalink / raw) To: cocci On Thu, Aug 23, 2018 at 2:48 PM, Joe Perches <joe@perches.com> wrote: > Forwarding a question about coccinelle and isomorphisms from Kees Cook > > ---------- Forwarded message ---------- > From: Kees Cook <keescook@chromium.org> > To: "Gustavo A. R. Silva" <gustavo@embeddedor.com> > Cc: Alessandro Zummo <a.zummo@towertech.it>, Alexandre Belloni <alexandre.belloni@bootlin.com>, Maxime Ripard <maxime.ripard@bootlin.com>, Chen-Yu Tsai <wens@csie.org>, linux-rtc at vger.kernel.org, linux-arm-kernel <linux-arm-kernel@lists.infradead.org>, LKML <linux-kernel@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@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@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? -Kees -- Kees Cook Pixel Security ^ permalink raw reply [flat|nested] 20+ messages in thread
* [Cocci] [Fwd: Re: [PATCH] rtc: sun6i: Use struct_size() in kzalloc()] 2018-08-23 21:56 ` [Cocci] [Fwd: Re: [PATCH] rtc: sun6i: Use struct_size() in kzalloc()] Kees Cook @ 2018-08-23 22:00 ` Julia Lawall 2018-08-23 22:06 ` Kees Cook 0 siblings, 1 reply; 20+ messages in thread From: Julia Lawall @ 2018-08-23 22:00 UTC (permalink / raw) To: cocci On Thu, 23 Aug 2018, Kees Cook wrote: > On Thu, Aug 23, 2018 at 2:48 PM, Joe Perches <joe@perches.com> wrote: > > Forwarding a question about coccinelle and isomorphisms from Kees Cook > > > > ---------- Forwarded message ---------- > > From: Kees Cook <keescook@chromium.org> > > To: "Gustavo A. R. Silva" <gustavo@embeddedor.com> > > Cc: Alessandro Zummo <a.zummo@towertech.it>, Alexandre Belloni <alexandre.belloni@bootlin.com>, Maxime Ripard <maxime.ripard@bootlin.com>, Chen-Yu Tsai <wens@csie.org>, linux-rtc at vger.kernel.org, linux-arm-kernel <linux-arm-kernel@lists.infradead.org>, LKML <linux-kernel@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@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@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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [Cocci] [Fwd: Re: [PATCH] rtc: sun6i: Use struct_size() in kzalloc()] 2018-08-23 22:00 ` Julia Lawall @ 2018-08-23 22:06 ` Kees Cook 2018-08-23 22:13 ` Julia Lawall 0 siblings, 1 reply; 20+ messages in thread From: Kees Cook @ 2018-08-23 22:06 UTC (permalink / raw) To: cocci On Thu, Aug 23, 2018 at 3:00 PM, Julia Lawall <julia.lawall@lip6.fr> wrote: > > > On Thu, 23 Aug 2018, Kees Cook wrote: > >> On Thu, Aug 23, 2018 at 2:48 PM, Joe Perches <joe@perches.com> wrote: >> > Forwarding a question about coccinelle and isomorphisms from Kees Cook >> > >> > ---------- Forwarded message ---------- >> > From: Kees Cook <keescook@chromium.org> >> > To: "Gustavo A. R. Silva" <gustavo@embeddedor.com> >> > Cc: Alessandro Zummo <a.zummo@towertech.it>, Alexandre Belloni <alexandre.belloni@bootlin.com>, Maxime Ripard <maxime.ripard@bootlin.com>, Chen-Yu Tsai <wens@csie.org>, linux-rtc at vger.kernel.org, linux-arm-kernel <linux-arm-kernel@lists.infradead.org>, LKML <linux-kernel@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@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@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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [Cocci] [Fwd: Re: [PATCH] rtc: sun6i: Use struct_size() in kzalloc()] 2018-08-23 22:06 ` Kees Cook @ 2018-08-23 22:13 ` Julia Lawall 2018-08-23 22:21 ` Joe Perches 0 siblings, 1 reply; 20+ messages in thread From: Julia Lawall @ 2018-08-23 22:13 UTC (permalink / raw) To: cocci On Thu, 23 Aug 2018, Kees Cook wrote: > On Thu, Aug 23, 2018 at 3:00 PM, Julia Lawall <julia.lawall@lip6.fr> wrote: > > > > > > On Thu, 23 Aug 2018, Kees Cook wrote: > > > >> On Thu, Aug 23, 2018 at 2:48 PM, Joe Perches <joe@perches.com> wrote: > >> > Forwarding a question about coccinelle and isomorphisms from Kees Cook > >> > > >> > ---------- Forwarded message ---------- > >> > From: Kees Cook <keescook@chromium.org> > >> > To: "Gustavo A. R. Silva" <gustavo@embeddedor.com> > >> > Cc: Alessandro Zummo <a.zummo@towertech.it>, Alexandre Belloni <alexandre.belloni@bootlin.com>, Maxime Ripard <maxime.ripard@bootlin.com>, Chen-Yu Tsai <wens@csie.org>, linux-rtc at vger.kernel.org, linux-arm-kernel <linux-arm-kernel@lists.infradead.org>, LKML <linux-kernel@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@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@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 I would say yes. Basically it removes the parentheses but doesn't reparse the code, so it doesn't redo the associativity. Since * has higher precedence than +, then both will be matched. On the other hand, if you put: (a + b) * c It will consider a pattern with the parentheses removed, but that pattern won't match anything, because real trees that consist of a + b * c are parsed in a different way. julia ^ permalink raw reply [flat|nested] 20+ messages in thread
* [Cocci] [Fwd: Re: [PATCH] rtc: sun6i: Use struct_size() in kzalloc()] 2018-08-23 22:13 ` Julia Lawall @ 2018-08-23 22:21 ` Joe Perches 2018-08-23 22:27 ` Kees Cook 0 siblings, 1 reply; 20+ messages in thread From: Joe Perches @ 2018-08-23 22:21 UTC (permalink / raw) To: cocci On Thu, 2018-08-23 at 18:13 -0400, Julia Lawall wrote: > > On Thu, 23 Aug 2018, Kees Cook wrote: > > > On Thu, Aug 23, 2018 at 3:00 PM, Julia Lawall <julia.lawall@lip6.fr> wrote: > > > > > > > > > On Thu, 23 Aug 2018, Kees Cook wrote: > > > > > > > On Thu, Aug 23, 2018 at 2:48 PM, Joe Perches <joe@perches.com> wrote: > > > > > Forwarding a question about coccinelle and isomorphisms from Kees Cook > > > > > > > > > > ---------- Forwarded message ---------- > > > > > From: Kees Cook <keescook@chromium.org> > > > > > To: "Gustavo A. R. Silva" <gustavo@embeddedor.com> > > > > > Cc: Alessandro Zummo <a.zummo@towertech.it>, Alexandre Belloni <alexandre.belloni@bootlin.com>, Maxime Ripard <maxime.ripard@bootlin.com>, Chen-Yu Tsai <wens@csie.org>, linux-rtc at vger.kernel.org, linux-arm-kernel <linux-arm-kernel@lists.infradead.org>, LKML <linux-kernel@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@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@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 > > I would say yes. Basically it removes the parentheses but doesn't reparse > the code, so it doesn't redo the associativity. Since * has higher > precedence than +, then both will be matched. On the other hand, if you > put: > > (a + b) * c > > It will consider a pattern with the parentheses removed, but that pattern > won't match anything, because real trees that consist of a + b * c are > parsed in a different way. spatch 1.0.4 doesn't seem to: $ spatch --version spatch version 1.0.4 with Python support and with PCRE support $ cat match_mul.cocci @@ expression a, b, c; int d; @@ * d = a * b + c; $ cat test_mul.c int a, b, c, d; void foo(void) { d = (a * b) + c; d = a * b + c; } $ spatch -sp-file match_mul.cocci test_mul.c init_defs_builtins: /usr/lib/coccinelle/standard.h HANDLING: test_mul.c diff = --- test_mul.c +++ /tmp/cocci-output-22607-ba6f76-test_mul.c @@ -3,5 +3,4 @@ int a, b, c, d; void foo(void) { d = (a * b) + c; - d = a * b + c; } ^ permalink raw reply [flat|nested] 20+ messages in thread
* [Cocci] [Fwd: Re: [PATCH] rtc: sun6i: Use struct_size() in kzalloc()] 2018-08-23 22:21 ` Joe Perches @ 2018-08-23 22:27 ` Kees Cook 2018-08-23 22:45 ` Julia Lawall 2018-08-23 22:59 ` Joe Perches 0 siblings, 2 replies; 20+ messages in thread From: Kees Cook @ 2018-08-23 22:27 UTC (permalink / raw) To: cocci On Thu, Aug 23, 2018 at 3:21 PM, Joe Perches <joe@perches.com> wrote: > On Thu, 2018-08-23 at 18:13 -0400, Julia Lawall wrote: >> >> On Thu, 23 Aug 2018, Kees Cook wrote: >> >> (a + b) * c >> >> It will consider a pattern with the parentheses removed, but that pattern >> won't match anything, because real trees that consist of a + b * c are >> parsed in a different way. > > spatch 1.0.4 doesn't seem to: > > $ spatch --version > spatch version 1.0.4 with Python support and with PCRE support > $ cat match_mul.cocci > @@ > expression a, b, c; > int d; > @@ > > * d = a * b + c; But "(a * b) + c" for the rule does: $ cat isomorphisms.c #include <stdio.h> #include <stdlib.h> int main(int argc, char *argv[]) { int a, b, c; a = atoi(argv[1]); b = atoi(argv[2]); c = atoi(argv[3]); printf("%d\n", a + b * c); printf("%d\n", (a + b) * c); printf("%d\n", a + (b * c)); printf("%d\n", b * c + a); printf("%d\n", b * (c + a)); printf("%d\n", (b * c) + a); return 0; } $ cat match.cocci @@ identifier A, B, C; @@ * (A * B) + C $ spatch -sp-file match.cocci isomorphisms.c init_defs_builtins: /usr/lib/coccinelle/standard.h HANDLING: isomorphisms.c diff = --- isomorphisms.c +++ /tmp/cocci-output-8402-870fd6-isomorphisms.c @@ -9,12 +9,8 @@ int main(int argc, char *argv[]) b = atoi(argv[2]); c = atoi(argv[3]); - printf("%d\n", a + b * c); printf("%d\n", (a + b) * c); - printf("%d\n", a + (b * c)); - printf("%d\n", b * c + a); printf("%d\n", b * (c + a)); - printf("%d\n", (b * c) + a); return 0; } So it sounds like I should put parens around all kinds of things in my rules... -Kees -- Kees Cook Pixel Security ^ permalink raw reply [flat|nested] 20+ messages in thread
* [Cocci] [Fwd: Re: [PATCH] rtc: sun6i: Use struct_size() in kzalloc()] 2018-08-23 22:27 ` Kees Cook @ 2018-08-23 22:45 ` Julia Lawall 2018-08-23 22:59 ` Joe Perches 1 sibling, 0 replies; 20+ messages in thread From: Julia Lawall @ 2018-08-23 22:45 UTC (permalink / raw) To: cocci On Thu, 23 Aug 2018, Kees Cook wrote: > On Thu, Aug 23, 2018 at 3:21 PM, Joe Perches <joe@perches.com> wrote: > > On Thu, 2018-08-23 at 18:13 -0400, Julia Lawall wrote: > >> > >> On Thu, 23 Aug 2018, Kees Cook wrote: > >> > >> (a + b) * c > >> > >> It will consider a pattern with the parentheses removed, but that pattern > >> won't match anything, because real trees that consist of a + b * c are > >> parsed in a different way. > > > > spatch 1.0.4 doesn't seem to: > > > > $ spatch --version > > spatch version 1.0.4 with Python support and with PCRE support > > $ cat match_mul.cocci > > @@ > > expression a, b, c; > > int d; > > @@ > > > > * d = a * b + c; > > But "(a * b) + c" for the rule does: > > > $ cat isomorphisms.c > #include <stdio.h> > #include <stdlib.h> > > int main(int argc, char *argv[]) > { > int a, b, c; > > a = atoi(argv[1]); > b = atoi(argv[2]); > c = atoi(argv[3]); > > printf("%d\n", a + b * c); > printf("%d\n", (a + b) * c); > printf("%d\n", a + (b * c)); > printf("%d\n", b * c + a); > printf("%d\n", b * (c + a)); > printf("%d\n", (b * c) + a); > > return 0; > } > $ cat match.cocci > @@ > identifier A, B, C; > @@ > > * (A * B) + C > > $ spatch -sp-file match.cocci isomorphisms.c > init_defs_builtins: /usr/lib/coccinelle/standard.h > HANDLING: isomorphisms.c > diff = > --- isomorphisms.c > +++ /tmp/cocci-output-8402-870fd6-isomorphisms.c > @@ -9,12 +9,8 @@ int main(int argc, char *argv[]) > b = atoi(argv[2]); > c = atoi(argv[3]); > > - printf("%d\n", a + b * c); > printf("%d\n", (a + b) * c); > - printf("%d\n", a + (b * c)); > - printf("%d\n", b * c + a); > printf("%d\n", b * (c + a)); > - printf("%d\n", (b * c) + a); > > return 0; > } > > > So it sounds like I should put parens around all kinds of things in my rules... If you think someone might reasonably use parentheses somewhere, you should put them. julia ^ permalink raw reply [flat|nested] 20+ messages in thread
* [Cocci] [Fwd: Re: [PATCH] rtc: sun6i: Use struct_size() in kzalloc()] 2018-08-23 22:27 ` Kees Cook 2018-08-23 22:45 ` Julia Lawall @ 2018-08-23 22:59 ` Joe Perches 2018-08-24 0:17 ` Julia Lawall 1 sibling, 1 reply; 20+ messages in thread From: Joe Perches @ 2018-08-23 22:59 UTC (permalink / raw) To: cocci On Thu, 2018-08-23 at 15:27 -0700, Kees Cook wrote: > On Thu, Aug 23, 2018 at 3:21 PM, Joe Perches <joe@perches.com> wrote: > > On Thu, 2018-08-23 at 18:13 -0400, Julia Lawall wrote: > > > > > > On Thu, 23 Aug 2018, Kees Cook wrote: > > > > > > (a + b) * c > > > > > > It will consider a pattern with the parentheses removed, but that pattern > > > won't match anything, because real trees that consist of a + b * c are > > > parsed in a different way. > > > > spatch 1.0.4 doesn't seem to: > > > > $ spatch --version > > spatch version 1.0.4 with Python support and with PCRE support > > $ cat match_mul.cocci > > @@ > > expression a, b, c; > > int d; > > @@ > > > > * d = a * b + c; > > But "(a * b) + c" for the rule does: Yes, but not for other valid uses like below: $ cat match_mul.cocci @@ expression a, b, c; int d; @@ * d = (a * b) + c; $ cat test_mul.c int a, b, c, d; void foo(void) { d = ((a * b) + c); d = ((a * b)) + c; d = (a * b) + c; d = a * b + c; } $ spatch -sp-file match_mul.cocci test_mul.c init_defs_builtins: /usr/lib/coccinelle/standard.h HANDLING: test_mul.c diff = --- test_mul.c +++ /tmp/cocci-output-23856-7e83f7-test_mul.c @@ -4,6 +4,4 @@ void foo(void) { d = ((a * b) + c); d = ((a * b)) + c; - d = (a * b) + c; - d = a * b + c; } ^ permalink raw reply [flat|nested] 20+ messages in thread
* [Cocci] [Fwd: Re: [PATCH] rtc: sun6i: Use struct_size() in kzalloc()] 2018-08-23 22:59 ` Joe Perches @ 2018-08-24 0:17 ` Julia Lawall 2018-08-24 0:25 ` Joe Perches 0 siblings, 1 reply; 20+ messages in thread From: Julia Lawall @ 2018-08-24 0:17 UTC (permalink / raw) To: cocci On Thu, 23 Aug 2018, Joe Perches wrote: > On Thu, 2018-08-23 at 15:27 -0700, Kees Cook wrote: > > On Thu, Aug 23, 2018 at 3:21 PM, Joe Perches <joe@perches.com> wrote: > > > On Thu, 2018-08-23 at 18:13 -0400, Julia Lawall wrote: > > > > > > > > On Thu, 23 Aug 2018, Kees Cook wrote: > > > > > > > > (a + b) * c > > > > > > > > It will consider a pattern with the parentheses removed, but that pattern > > > > won't match anything, because real trees that consist of a + b * c are > > > > parsed in a different way. > > > > > > spatch 1.0.4 doesn't seem to: > > > > > > $ spatch --version > > > spatch version 1.0.4 with Python support and with PCRE support > > > $ cat match_mul.cocci > > > @@ > > > expression a, b, c; > > > int d; > > > @@ > > > > > > * d = a * b + c; > > > > But "(a * b) + c" for the rule does: > > Yes, but not for other valid uses like below: > > $ cat match_mul.cocci > @@ > expression a, b, c; > int d; > @@ > > * d = (a * b) + c; > $ cat test_mul.c > int a, b, c, d; > > void foo(void) > { > d = ((a * b) + c); > d = ((a * b)) + c; > d = (a * b) + c; > d = a * b + c; > } > $ spatch -sp-file match_mul.cocci test_mul.c > init_defs_builtins: /usr/lib/coccinelle/standard.h > HANDLING: test_mul.c > diff = > --- test_mul.c > +++ /tmp/cocci-output-23856-7e83f7-test_mul.c > @@ -4,6 +4,4 @@ void foo(void) > { > d = ((a * b) + c); > d = ((a * b)) + c; > - d = (a * b) + c; > - d = a * b + c; > } I'm not sure to get the point. If you think that people may fully parenthesize the code, then ((a * b) + c) would be a reasonable option for the pattern. I'm not sure why double parentheses, ((a * b)) + c, should exist in the first place. julia ^ permalink raw reply [flat|nested] 20+ messages in thread
* [Cocci] [Fwd: Re: [PATCH] rtc: sun6i: Use struct_size() in kzalloc()] 2018-08-24 0:17 ` Julia Lawall @ 2018-08-24 0:25 ` Joe Perches 0 siblings, 0 replies; 20+ messages in thread From: Joe Perches @ 2018-08-24 0:25 UTC (permalink / raw) To: cocci On Thu, 2018-08-23 at 20:17 -0400, Julia Lawall wrote: > > On Thu, 23 Aug 2018, Joe Perches wrote: > > > On Thu, 2018-08-23 at 15:27 -0700, Kees Cook wrote: > > > On Thu, Aug 23, 2018 at 3:21 PM, Joe Perches <joe@perches.com> wrote: > > > > On Thu, 2018-08-23 at 18:13 -0400, Julia Lawall wrote: > > > > > > > > > > On Thu, 23 Aug 2018, Kees Cook wrote: > > > > > > > > > > (a + b) * c > > > > > > > > > > It will consider a pattern with the parentheses removed, but that pattern > > > > > won't match anything, because real trees that consist of a + b * c are > > > > > parsed in a different way. > > > > > > > > spatch 1.0.4 doesn't seem to: > > > > > > > > $ spatch --version > > > > spatch version 1.0.4 with Python support and with PCRE support > > > > $ cat match_mul.cocci > > > > @@ > > > > expression a, b, c; > > > > int d; > > > > @@ > > > > > > > > * d = a * b + c; > > > > > > But "(a * b) + c" for the rule does: > > > > Yes, but not for other valid uses like below: > > > > $ cat match_mul.cocci > > @@ > > expression a, b, c; > > int d; > > @@ > > > > * d = (a * b) + c; > > $ cat test_mul.c > > int a, b, c, d; > > > > void foo(void) > > { > > d = ((a * b) + c); > > d = ((a * b)) + c; > > d = (a * b) + c; > > d = a * b + c; > > } > > $ spatch -sp-file match_mul.cocci test_mul.c > > init_defs_builtins: /usr/lib/coccinelle/standard.h > > HANDLING: test_mul.c > > diff = > > --- test_mul.c > > +++ /tmp/cocci-output-23856-7e83f7-test_mul.c > > @@ -4,6 +4,4 @@ void foo(void) > > { > > d = ((a * b) + c); > > d = ((a * b)) + c; > > - d = (a * b) + c; > > - d = a * b + c; > > } > > I'm not sure to get the point. > > If you think that people may fully parenthesize the code, then ((a * b) + > c) would be a reasonable option for the pattern. I'm not sure why double > parentheses, ((a * b)) + c, should exist in the first place. People who write code do a lot of unnecessary things. It's code I've seen in the past. It occurs more with expressions than identifiers. It'd be nice if coccinelle could parse arbitrarily parenthesized code and script down to its minimal logical requirements and match as maximally as possible. cheers, Joe ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] rtc: sun6i: Use struct_size() in kzalloc() 2018-08-23 20:56 ` Kees Cook @ 2018-08-23 22:01 ` Gustavo A. R. Silva -1 siblings, 0 replies; 20+ messages in thread From: Gustavo A. R. Silva @ 2018-08-23 22:01 UTC (permalink / raw) To: Kees Cook Cc: Alessandro Zummo, Alexandre Belloni, Maxime Ripard, Chen-Yu Tsai, linux-rtc, linux-arm-kernel, LKML On 8/23/18 3:56 PM, Kees Cook wrote: >> >> - 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? > Apparently. If I manually remove the ()s, the cocci script successfully generates a patch. -- Gustavo ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH] rtc: sun6i: Use struct_size() in kzalloc() @ 2018-08-23 22:01 ` Gustavo A. R. Silva 0 siblings, 0 replies; 20+ messages in thread From: Gustavo A. R. Silva @ 2018-08-23 22:01 UTC (permalink / raw) To: linux-arm-kernel On 8/23/18 3:56 PM, Kees Cook wrote: >> >> - 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? > Apparently. If I manually remove the ()s, the cocci script successfully generates a patch. -- Gustavo ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] rtc: sun6i: Use struct_size() in kzalloc() 2018-08-23 18:51 ` Gustavo A. R. Silva @ 2018-08-23 21:56 ` Kees Cook -1 siblings, 0 replies; 20+ messages in thread From: Kees Cook @ 2018-08-23 21:56 UTC (permalink / raw) To: Gustavo A. R. Silva Cc: Alessandro Zummo, Alexandre Belloni, Maxime Ripard, Chen-Yu Tsai, linux-rtc, linux-arm-kernel, LKML On Thu, Aug 23, 2018 at 11:51 AM, Gustavo A. R. Silva <gustavo@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@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; Reviewed-by: Kees Cook <keescook@chromium.org> -Kees -- Kees Cook Pixel Security ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH] rtc: sun6i: Use struct_size() in kzalloc() @ 2018-08-23 21:56 ` Kees Cook 0 siblings, 0 replies; 20+ messages in thread From: Kees Cook @ 2018-08-23 21:56 UTC (permalink / raw) To: linux-arm-kernel On Thu, Aug 23, 2018 at 11:51 AM, Gustavo A. R. Silva <gustavo@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@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; Reviewed-by: Kees Cook <keescook@chromium.org> -Kees -- Kees Cook Pixel Security ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] rtc: sun6i: Use struct_size() in kzalloc() 2018-08-23 18:51 ` Gustavo A. R. Silva @ 2018-08-27 21:02 ` Alexandre Belloni -1 siblings, 0 replies; 20+ messages in thread From: Alexandre Belloni @ 2018-08-27 21:02 UTC (permalink / raw) To: Gustavo A. R. Silva Cc: Alessandro Zummo, Maxime Ripard, Chen-Yu Tsai, linux-rtc, linux-arm-kernel, linux-kernel, Kees Cook On 23/08/2018 13:51:40-0500, Gustavo A. R. Silva 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@embeddedor.com> > --- > drivers/rtc/rtc-sun6i.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > Applied, thanks. -- Alexandre Belloni, Bootlin Embedded Linux and Kernel engineering https://bootlin.com ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH] rtc: sun6i: Use struct_size() in kzalloc() @ 2018-08-27 21:02 ` Alexandre Belloni 0 siblings, 0 replies; 20+ messages in thread From: Alexandre Belloni @ 2018-08-27 21:02 UTC (permalink / raw) To: linux-arm-kernel On 23/08/2018 13:51:40-0500, Gustavo A. R. Silva 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@embeddedor.com> > --- > drivers/rtc/rtc-sun6i.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > Applied, thanks. -- Alexandre Belloni, Bootlin Embedded Linux and Kernel engineering https://bootlin.com ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2018-08-28 0:51 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-08-23 18:51 [PATCH] rtc: sun6i: Use struct_size() in kzalloc() Gustavo A. R. Silva
2018-08-23 18:51 ` Gustavo A. R. Silva
2018-08-23 20:56 ` Kees Cook
2018-08-23 20:56 ` Kees Cook
[not found] ` <700bfdf296aa112799b6a6f091162cc34f6fef10.camel@perches.com>
2018-08-23 21:56 ` [Cocci] [Fwd: Re: [PATCH] rtc: sun6i: Use struct_size() in kzalloc()] Kees Cook
2018-08-23 22:00 ` Julia Lawall
2018-08-23 22:06 ` Kees Cook
2018-08-23 22:13 ` Julia Lawall
2018-08-23 22:21 ` Joe Perches
2018-08-23 22:27 ` Kees Cook
2018-08-23 22:45 ` Julia Lawall
2018-08-23 22:59 ` Joe Perches
2018-08-24 0:17 ` Julia Lawall
2018-08-24 0:25 ` Joe Perches
2018-08-23 22:01 ` [PATCH] rtc: sun6i: Use struct_size() in kzalloc() Gustavo A. R. Silva
2018-08-23 22:01 ` Gustavo A. R. Silva
2018-08-23 21:56 ` Kees Cook
2018-08-23 21:56 ` Kees Cook
2018-08-27 21:02 ` Alexandre Belloni
2018-08-27 21:02 ` Alexandre Belloni
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.