Skip to contentSkip to conversation

fbdev/fb.h: Use struct_size() helper in kzalloc()

PATCH Applied Gustavo A. R. Silva 3 participants 4 messages Jun 17, 2020 – Jul 10, 2020 Framebuffer layer

lore
Share & tools
lore

Gustavo A. R. Silva

author
#0 lore raw

PATCH 1/1 fbdev/fb.h: Use struct_size() helper in kzalloc()

Make use of the struct_size() helper instead of an open-coded version in order to avoid any potential type mistakes.

This code was detected with the help of Coccinelle and, audited and fixed manually.

Signed-off-by Gustavo A. R. Silva <[email protected]>
---
 include/linux/fb.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
include/linux/fb.h +3 -2
index 3b4b2f0c6994..2b530e6d86e4 100644
@@ -506,8 +506,9 @@ struct fb_info {
 };
 
 static inline struct apertures_struct *alloc_apertures(unsigned int max_num) {
-	struct apertures_struct *a = kzalloc(sizeof(struct apertures_struct)
-			+ max_num * sizeof(struct aperture), GFP_KERNEL);
+	struct apertures_struct *a;
+
+	a = kzalloc(struct_size(a, ranges, max_num), GFP_KERNEL);
 	if (!a)
 		return NULL;
 	a->count = max_num;
Signature · 6 lines
2.27.0

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
#1 lore raw

Hi Gustavo.

Quoted 7 lines · Gustavo A. R. Silva

Make use of the struct_size() helper instead of an open-coded version in order to avoid any potential type mistakes.

This code was detected with the help of Coccinelle and, audited and fixed manually.

Signed-off-by Gustavo A. R. Silva <[email protected]>

struct_size is defined in overflow.h - which is not included by fs.h. So we rely on overflow.h being pulled in by some other header - maybe slab.h in this case.
Seems fragile, should this patch add an include of overflow.h?

Sam

Quoted 27 lines
---
 include/linux/fb.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
include/linux/fb.h +3 -2
index 3b4b2f0c6994..2b530e6d86e4 100644
@@ -506,8 +506,9 @@ struct fb_info {
 };
 
 static inline struct apertures_struct *alloc_apertures(unsigned int max_num) {
-	struct apertures_struct *a = kzalloc(sizeof(struct apertures_struct)
-			+ max_num * sizeof(struct aperture), GFP_KERNEL);
+	struct apertures_struct *a;
+
+	a = kzalloc(struct_size(a, ranges, max_num), GFP_KERNEL);
 	if (!a)
 		return NULL;
 	a->count = max_num;
Signature · 6 lines
2.27.0

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Applied
#3 lore raw
In reply to Gustavo A. R. Silva
Quoted 7 lines · Gustavo A. R. Silva

Make use of the struct_size() helper instead of an open-coded version in order to avoid any potential type mistakes.

This code was detected with the help of Coccinelle and, audited and fixed manually.

Signed-off-by Gustavo A. R. Silva <[email protected]>

Applied to drm-misc-next tree, thanks.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Quoted 21 lines
---
 include/linux/fb.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
include/linux/fb.h +3 -2
index 3b4b2f0c6994..2b530e6d86e4 100644
@@ -506,8 +506,9 @@ struct fb_info {
 };
 
 static inline struct apertures_struct *alloc_apertures(unsigned int max_num) {
-	struct apertures_struct *a = kzalloc(sizeof(struct apertures_struct)
-			+ max_num * sizeof(struct aperture), GFP_KERNEL);
+	struct apertures_struct *a;
+
+	a = kzalloc(struct_size(a, ranges, max_num), GFP_KERNEL);
 	if (!a)
 		return NULL;
 	a->count = max_num;
#2 lore raw
In reply to Sam Ravnborg
Quoted 9 lines · Sam Ravnborg

Hi Gustavo.

Quoted 6 lines · Gustavo A. R. Silva

Make use of the struct_size() helper instead of an open-coded version in order to avoid any potential type mistakes.

This code was detected with the help of Coccinelle and, audited and fixed manually.

Quoted 6 lines
Quoted 1 line
Signed-off-by Gustavo A. R. Silva <[email protected]>

struct_size is defined in overflow.h - which is not included by fs.h. So we rely on overflow.h being pulled in by some other header - maybe slab.h in this case.
Seems fragile, should this patch add an include of overflow.h?

$ git grep struct_size drivers/|wc -l
697
$ git grep overflow\\.h drivers/|wc -l
8
$ git grep overflow\\.h include/linux/
include/linux/device.h:#include <linux/overflow.h>
include/linux/mm.h:#include <linux/overflow.h>
include/linux/slab.h:#include <linux/overflow.h>
include/linux/vmalloc.h:#include <linux/overflow.h>

so I've applied the patch as it is (hoping that the issue is so widespread that no-one tries to remove overflow.h from slab.h without fixing drivers at the same time)..

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Quoted 29 lines

Sam

Quoted 27 lines
---
 include/linux/fb.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
include/linux/fb.h +3 -2
index 3b4b2f0c6994..2b530e6d86e4 100644
@@ -506,8 +506,9 @@ struct fb_info {
 };
 
 static inline struct apertures_struct *alloc_apertures(unsigned int max_num) {
-	struct apertures_struct *a = kzalloc(sizeof(struct apertures_struct)
-			+ max_num * sizeof(struct aperture), GFP_KERNEL);
+	struct apertures_struct *a;
+
+	a = kzalloc(struct_size(a, ranges, max_num), GFP_KERNEL);
 	if (!a)
 		return NULL;
 	a->count = max_num;
Signature · 6 lines
2.27.0

_______________________________________________
dri-devel mailing list
[email protected]
https://protect2.fireeye.com/url?k{ae4d09-26604cda-7bafc646-000babff317b-7eab3a2caa4b8b73&q=1&u=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel