Skip to contentSkip to conversation

staging: fbtft: Prefer using kmalloc_array

PATCH In review Bhaktipriya Shridhar 2 participants 4 messages Feb 21, 2016

Subsystems (2)
Staging Fbtft Framebuffer drivers
lore
Share & tools
lore

Bhaktipriya Shridhar

author
#0 lore raw

PATCH 1/1 staging: fbtft: Prefer using kmalloc_array

Use kmalloc_array instead of kmalloc for arrays to prevent integer overflows.
This was done using Coccinelle:
@@
expression e1, e2;
constant C;
type t;
@@
(
- kmalloc(sizeof(e1)*e2, C)
+ kmalloc_array(e2, sizeof(e1), C)
|
- kmalloc(e2*sizeof(e1), C)
+ kmalloc_array(e2, sizeof(e1), C)
|
- kmalloc(e2*sizeof(t), C)
+ kmalloc_array(e2, sizeof(t), C)
|
- kmalloc(sizeof(t)*e2, C)
+ kmalloc_array(e2, sizeof(t), C)
)

Signed-off-by Bhaktipriya Shridhar <[email protected]>
---
 drivers/staging/fbtft/fb_agm1264k-fl.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
drivers/staging/fbtft/fb_agm1264k-fl.c +3 -2
index 2a50cf9..5fbfd50 100644
@@ -272,8 +272,9 @@ static int write_vmem(struct fbtft_par *par, size_t offset, size_t len)
 	int ret = 0;
 
 	/* buffer to convert RGB565 -> grayscale16 -> Dithered image 1bpp */
-	signed short *convert_buf = kmalloc(par->info->var.xres *
-		par->info->var.yres * sizeof(signed short), GFP_NOIO);
+	signed short *convert_buf = kmalloc_array(par->info->var.xres * par->info->var.yres,
+						  sizeof(signed short),
+						  GFP_NOIO);
 
 	if (!convert_buf)
 		return -ENOMEM;

--
2.1.4

#1 lore raw
Quoted 32 lines · Bhaktipriya Shridhar

Use kmalloc_array instead of kmalloc for arrays to prevent integer overflows.
This was done using Coccinelle:
@@
expression e1, e2;
constant C;
type t;
@@
(
- kmalloc(sizeof(e1)*e2, C)
+ kmalloc_array(e2, sizeof(e1), C)
|
- kmalloc(e2*sizeof(e1), C)
+ kmalloc_array(e2, sizeof(e1), C)
|
- kmalloc(e2*sizeof(t), C)
+ kmalloc_array(e2, sizeof(t), C)
|
- kmalloc(sizeof(t)*e2, C)
+ kmalloc_array(e2, sizeof(t), C)
)

Signed-off-by Bhaktipriya Shridhar <[email protected]>
---
 drivers/staging/fbtft/fb_agm1264k-fl.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
drivers/staging/fbtft/fb_agm1264k-fl.c +0 -0
index 2a50cf9..5fbfd50 100644

drivers/staging/fbtft/fb_agm1264k-fl.c static int write_vmem(struct fbtft_par *par, size_t offset, size_t len) · line 277

Quoted 8 lines
drivers/staging/fbtft/fb_agm1264k-fl.c +3 -2
@@ -272,8 +272,9 @@ static int write_vmem(struct fbtft_par *par, size_t offset, size_t len)
 	int ret = 0;
 
 	/* buffer to convert RGB565 -> grayscale16 -> Dithered image 1bpp */
-	signed short *convert_buf = kmalloc(par->info->var.xres *
-		par->info->var.yres * sizeof(signed short), GFP_NOIO);
+	signed short *convert_buf = kmalloc_array(par->info->var.xres * par->info->var.yres,
+						  sizeof(signed short),
+						  GFP_NOIO);

You should adjust the output of Coccinelle so as to not introduce 80 character violations.

julia

Quoted 13 lines
drivers/staging/fbtft/fb_agm1264k-fl.c +0 -1
 	if (!convert_buf)
 		return -ENOMEM;
--

2.1.4

--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20160221113832.GA28242%40Karyakshetra.
For more options, visit https://groups.google.com/d/optout.

author
#2 lore raw

Hello Julia,

Thank you for your feedback. I'll send a v2 for this patch. Do you recommend going for the following semantic patch?

It is preserving the format of the original file and is not introducing
any 80 character violations.

@@
expression e1, e2;
constant C;
type t;
@@
(
- kmalloc(
+ kmalloc_array(
e1
- *
+ ,
sizeof(e1), C)
|
- kmalloc(
+ kmalloc_array(
e2
- *
+ ,
sizeof(t), C)
|
- kmalloc(
+ kmalloc_array(
- sizeof(e1)
- *
e2
+ ,
+ sizeof(e1)
, C)
|
- kmalloc(
+ kmalloc_array(
- sizeof(e1)
- *
e2
+ ,
+ sizeof(t)
, C)
)

Quoted 70 lines · Julia Lawall
Quoted 22 lines · Bhaktipriya Shridhar

Use kmalloc_array instead of kmalloc for arrays to prevent integer overflows.
This was done using Coccinelle:
@@
expression e1, e2;
constant C;
type t;
@@
(
- kmalloc(sizeof(e1)*e2, C)
+ kmalloc_array(e2, sizeof(e1), C)
|
- kmalloc(e2*sizeof(e1), C)
+ kmalloc_array(e2, sizeof(e1), C)
|
- kmalloc(e2*sizeof(t), C)
+ kmalloc_array(e2, sizeof(t), C)
|
- kmalloc(sizeof(t)*e2, C)
+ kmalloc_array(e2, sizeof(t), C)
)

Review

Quoted 6 lines
Signed-off-by Bhaktipriya Shridhar <[email protected]>
---
 drivers/staging/fbtft/fb_agm1264k-fl.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
(quoted diff) +0 -0

b/drivers/staging/fbtft/fb_agm1264k-fl.c

drivers/staging/fbtft/fb_agm1264k-fl.c static int write_vmem(struct fbtft_par *par, size_t

Quoted 4 lines

index 2a50cf9..5fbfd50 100644

drivers/staging/fbtft/fb_agm1264k-fl.c +0 -0
@@ -272,8 +272,9 @@ static int write_vmem(struct fbtft_par *par, size_t

offset, size_t len)

drivers/staging/fbtft/fb_agm1264k-fl.c static int write_vmem(struct fbtft_par *par, size_t offset, size_t len)

Quoted 3 lines
drivers/staging/fbtft/fb_agm1264k-fl.c +0 -0
      int ret = 0;
 
      /* buffer to convert RGB565 -> grayscale16 -> Dithered image 1bpp

*/

drivers/staging/fbtft/fb_agm1264k-fl.c static int write_vmem(struct fbtft_par *par, size_t offset, size_t len)

Quoted 3 lines
drivers/staging/fbtft/fb_agm1264k-fl.c +1 -2
-     signed short *convert_buf = kmalloc(par->info->var.xres *
-             par->info->var.yres * sizeof(signed short), GFP_NOIO);
+     signed short *convert_buf = kmalloc_array(par->info->var.xres *

par->info->var.yres,

drivers/staging/fbtft/fb_agm1264k-fl.c static int write_vmem(struct fbtft_par *par, size_t offset, size_t len) · line 277

Quoted 2 lines
drivers/staging/fbtft/fb_agm1264k-fl.c +2 -0
+                                               sizeof(signed short),
+                                               GFP_NOIO);

You should adjust the output of Coccinelle so as to not introduce 80 character violations.

julia

Quoted 8 lines
drivers/staging/fbtft/fb_agm1264k-fl.c +0 -1
      if (!convert_buf)
              return -ENOMEM;
--

2.1.4

--
You received this message because you are subscribed to the Google

Groups "outreachy-kernel" group.

Quoted 1 line

To unsubscribe from this group and stop receiving emails from it, send

an email to [email protected].

Quoted 2 lines

To post to this group, send email to [email protected]. To view this discussion on the web visit

https://groups.google.com/d/msgid/outreachy-kernel/20160221113832.GA28242%40Karyakshetra
.

Quoted 2 lines

For more options, visit https://groups.google.com/d/optout.

#3 lore raw
Quoted 9 lines · Bhaktipriya Shridhar

Hello Julia,

Thank you for your feedback.  I'll send a v2 for this patch. Do you recommend going for the following semantic patch?

 It is preserving the format of the original file and is not introducing any  80 character violations.

Quoted 12 lines

@@
expression e1, e2;
constant C;
type t;
@@
(
- kmalloc(
+ kmalloc_array(
e1
- *
+ ,
sizeof(e1), C)

I think you mean e2 here?

In general, the thing that you want to avoid transforming is the case where the non-sizeof expression is a constant. So you can make a separate element of the disjunction, and put it at the beginning with no transformation annotation. Coccinelle takes the elements of a disjunction in order, so if you have a pattern that comes early with no transformation on it, then no transformation will happen in that case.

julia

Quoted 116 lines

|
- kmalloc(
+ kmalloc_array(
e2
- *
+ ,
sizeof(t), C)
|
- kmalloc(
+ kmalloc_array(
- sizeof(e1)
- *
e2
+ ,
+ sizeof(e1)
, C)
|
- kmalloc(
+ kmalloc_array(
- sizeof(e1)
- *
e2
+ ,
+ sizeof(t)
, C)
)

On Sun, Feb 21, 2016 at 5:53 PM, Julia Lawall <[email protected]> wrote:

      On Sun, 21 Feb 2016, Bhaktipriya Shridhar wrote:

      > Use kmalloc_array instead of kmalloc for arrays to prevent
      integer
      > overflows.
      > This was done using Coccinelle:
      > @@
      > expression e1, e2;
      > constant C;
      > type t;
      > @@
      > (
      > - kmalloc(sizeof(e1)*e2, C)
      > + kmalloc_array(e2, sizeof(e1), C)
      > |
      > - kmalloc(e2*sizeof(e1), C)
      > + kmalloc_array(e2, sizeof(e1), C)
      > |
      > - kmalloc(e2*sizeof(t), C)
      > + kmalloc_array(e2, sizeof(t), C)
      > |
      > - kmalloc(sizeof(t)*e2, C)
      > + kmalloc_array(e2, sizeof(t), C)
      > )
      >
      > Signed-off-by: Bhaktipriya Shridhar <[email protected]>
      > ---
      >  drivers/staging/fbtft/fb_agm1264k-fl.c | 5 +++--
      >  1 file changed, 3 insertions(+), 2 deletions(-)
      >
      > diff --git a/drivers/staging/fbtft/fb_agm1264k-fl.c
      b/drivers/staging/fbtft/fb_agm1264k-fl.c
      > index 2a50cf9..5fbfd50 100644
      > --- a/drivers/staging/fbtft/fb_agm1264k-fl.c
      > +++ b/drivers/staging/fbtft/fb_agm1264k-fl.c
      > @@ -272,8 +272,9 @@ static int write_vmem(struct fbtft_par
      *par, size_t offset, size_t len)
      >       int ret = 0;
      >
      >       /* buffer to convert RGB565 -> grayscale16 -> Dithered
      image 1bpp */
      > -     signed short *convert_buf = kmalloc(par->info->var.xres
      *
      > -             par->info->var.yres * sizeof(signed short),
      GFP_NOIO);
      > +     signed short *convert_buf =
      kmalloc_array(par->info->var.xres * par->info->var.yres,
      > +                                               sizeof(signed
      short),
      > +                                               GFP_NOIO);

      You should adjust the output of Coccinelle so as to not
      introduce 80
      character violations.

      julia

      >
      >       if (!convert_buf)
      >               return -ENOMEM;
      > --
      > 2.1.4
      >
      > --
      > You received this message because you are subscribed to the
      Google Groups "outreachy-kernel" group.
      > To unsubscribe from this group and stop receiving emails from
      it, send an email to
      [email protected].
      > To post to this group, send email to
      [email protected].
      > To view this discussion on the web visithttps://groups.google.com/d/msgid/outreachy-kernel/20160221113832.GA28242%4
      0Karyakshetra.
      > For more options, visit https://groups.google.com/d/optout.
      >

--
You received this message because you are subscribed to the Google Groups
"outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an
email to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visithttps://groups.google.com/d/msgid/outreachy-kernel/CAFNHFT9kXBxJbFKapk%2B-m QNx2-pgnYqwfokM0-00Lqw0pk6Low%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.