| Message ID | 20230531043826.991183-1-suhui@nfschina.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/nouveau/nvif: use struct_size() | expand |
On Wed, May 31, 2023 at 12:38:26PM +0800, Su Hui wrote: > Use struct_size() instead of hand writing it. > This is less verbose and more informative. > > Signed-off-by: Su Hui <suhui@nfschina.com> > --- > drivers/gpu/drm/nouveau/nvif/object.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/nouveau/nvif/object.c b/drivers/gpu/drm/nouveau/nvif/object.c > index 4d1aaee8fe15..4bd693aa4ee0 100644 > --- a/drivers/gpu/drm/nouveau/nvif/object.c > +++ b/drivers/gpu/drm/nouveau/nvif/object.c > @@ -65,7 +65,7 @@ nvif_object_sclass_get(struct nvif_object *object, struct nvif_sclass **psclass) > u32 size; > > while (1) { > - size = sizeof(*args) + cnt * sizeof(args->sclass.oclass[0]); > + size = struct_size(args, sclass.oclass, cnt); This is from the original code, but now that you are using the struct_size() macro static checkers will complain about it. (Maybe they don't yet?). size is a u32. Never save struct_size() returns to anything except unsigned long or size_t. (ssize_t is also fine, I suppose). Otherwise, you do not benefit from the integer overflow checking. > if (!(args = kmalloc(size, GFP_KERNEL))) > return -ENOMEM; > args->ioctl.version = 0; regards, dan carpenter
On 2023/5/31 16:31, Dan Carpenter wrote: > On Wed, May 31, 2023 at 12:38:26PM +0800, Su Hui wrote: >> Use struct_size() instead of hand writing it. >> This is less verbose and more informative. >> >> Signed-off-by: Su Hui <suhui@nfschina.com> >> --- >> drivers/gpu/drm/nouveau/nvif/object.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/nouveau/nvif/object.c b/drivers/gpu/drm/nouveau/nvif/object.c >> index 4d1aaee8fe15..4bd693aa4ee0 100644 >> --- a/drivers/gpu/drm/nouveau/nvif/object.c >> +++ b/drivers/gpu/drm/nouveau/nvif/object.c >> @@ -65,7 +65,7 @@ nvif_object_sclass_get(struct nvif_object *object, struct nvif_sclass **psclass) >> u32 size; >> >> while (1) { >> - size = sizeof(*args) + cnt * sizeof(args->sclass.oclass[0]); >> + size = struct_size(args, sclass.oclass, cnt); > This is from the original code, but now that you are using the > struct_size() macro static checkers will complain about it. (Maybe they > don't yet?). size is a u32. Never save struct_size() returns to > anything except unsigned long or size_t. (ssize_t is also fine, I > suppose). Otherwise, you do not benefit from the integer overflow > checking. Sorry, I don't notice the issue caused by type size. You are right, this patch is wrong because of the type mismatch. Thanks for your reply! Su Hui >> if (!(args = kmalloc(size, GFP_KERNEL))) >> return -ENOMEM; >> args->ioctl.version = 0; > regards, > dan carpenter
diff --git a/drivers/gpu/drm/nouveau/nvif/object.c b/drivers/gpu/drm/nouveau/nvif/object.c index 4d1aaee8fe15..4bd693aa4ee0 100644 --- a/drivers/gpu/drm/nouveau/nvif/object.c +++ b/drivers/gpu/drm/nouveau/nvif/object.c @@ -65,7 +65,7 @@ nvif_object_sclass_get(struct nvif_object *object, struct nvif_sclass **psclass) u32 size; while (1) { - size = sizeof(*args) + cnt * sizeof(args->sclass.oclass[0]); + size = struct_size(args, sclass.oclass, cnt); if (!(args = kmalloc(size, GFP_KERNEL))) return -ENOMEM; args->ioctl.version = 0;
Use struct_size() instead of hand writing it. This is less verbose and more informative. Signed-off-by: Su Hui <suhui@nfschina.com> --- drivers/gpu/drm/nouveau/nvif/object.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)