From: "Gustavo A. R. Silva" <gustavoars@kernel.org>
To: "Alex Deucher" <alexander.deucher@amd.com>,
"Christian König" <christian.koenig@amd.com>,
"David Airlie" <airlied@linux.ie>,
"Daniel Vetter" <daniel@ffwll.ch>
Cc: "Gustavo A. R. Silva" <gustavo@embeddedor.com>,
dri-devel@lists.freedesktop.org, amd-gfx@lists.freedesktop.org,
linux-kernel@vger.kernel.org
Subject: [PATCH] drm/radeon/dpm: Replace one-element array and use struct_size() helper
Date: Thu, 21 May 2020 20:25:47 -0500 [thread overview]
Message-ID: <20200522012547.GA6783@embeddedor> (raw)
The current codebase makes use of one-element arrays in the following
form:
struct something {
int length;
u8 data[1];
};
struct something *instance;
instance = kmalloc(sizeof(*instance) + size, GFP_KERNEL);
instance->length = size;
memcpy(instance->data, source, size);
but the preferred mechanism to declare variable-length types such as
these ones is a flexible array member[1][2], introduced in C99:
struct foo {
int stuff;
struct boo array[];
};
By making use of the mechanism above, we will get a compiler warning
in case the flexible array does not occur last in the structure, which
will help us prevent some kind of undefined behavior bugs from being
inadvertently introduced[3] to the codebase from now on. So, replace
the one-element array with a flexible-array member.
Also, make use of the new struct_size() helper to properly calculate the
size of struct NISLANDS_SMC_SWSTATE.
This issue was found with the help of Coccinelle and, audited and fixed
_manually_.
[1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
[2] https://github.com/KSPP/linux/issues/21
[3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour")
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
---
drivers/gpu/drm/amd/amdgpu/si_dpm.h | 2 +-
drivers/gpu/drm/radeon/ni_dpm.c | 7 ++++---
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/si_dpm.h b/drivers/gpu/drm/amd/amdgpu/si_dpm.h
index 6b7d292b919f3..bc0be6818e218 100644
--- a/drivers/gpu/drm/amd/amdgpu/si_dpm.h
+++ b/drivers/gpu/drm/amd/amdgpu/si_dpm.h
@@ -781,7 +781,7 @@ struct NISLANDS_SMC_SWSTATE
uint8_t levelCount;
uint8_t padding2;
uint8_t padding3;
- NISLANDS_SMC_HW_PERFORMANCE_LEVEL levels[1];
+ NISLANDS_SMC_HW_PERFORMANCE_LEVEL levels[];
};
typedef struct NISLANDS_SMC_SWSTATE NISLANDS_SMC_SWSTATE;
diff --git a/drivers/gpu/drm/radeon/ni_dpm.c b/drivers/gpu/drm/radeon/ni_dpm.c
index b57c37ddd164c..7d81dde509dc9 100644
--- a/drivers/gpu/drm/radeon/ni_dpm.c
+++ b/drivers/gpu/drm/radeon/ni_dpm.c
@@ -2685,11 +2685,12 @@ static int ni_upload_sw_state(struct radeon_device *rdev,
struct rv7xx_power_info *pi = rv770_get_pi(rdev);
u16 address = pi->state_table_start +
offsetof(NISLANDS_SMC_STATETABLE, driverState);
- u16 state_size = sizeof(NISLANDS_SMC_SWSTATE) +
- ((NISLANDS_MAX_SMC_PERFORMANCE_LEVELS_PER_SWSTATE - 1) * sizeof(NISLANDS_SMC_HW_PERFORMANCE_LEVEL));
+ NISLANDS_SMC_SWSTATE *smc_state;
+ u16 state_size = struct_size(smc_state, levels,
+ NISLANDS_MAX_SMC_PERFORMANCE_LEVELS_PER_SWSTATE);
int ret;
- NISLANDS_SMC_SWSTATE *smc_state = kzalloc(state_size, GFP_KERNEL);
+ smc_state = kzalloc(state_size, GFP_KERNEL);
if (smc_state == NULL)
return -ENOMEM;
--
2.26.2
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
WARNING: multiple messages have this Message-ID (diff)
From: "Gustavo A. R. Silva" <gustavoars@kernel.org>
To: "Alex Deucher" <alexander.deucher@amd.com>,
"Christian König" <christian.koenig@amd.com>,
"David Airlie" <airlied@linux.ie>,
"Daniel Vetter" <daniel@ffwll.ch>
Cc: "Gustavo A. R. Silva" <gustavo@embeddedor.com>,
dri-devel@lists.freedesktop.org, amd-gfx@lists.freedesktop.org,
linux-kernel@vger.kernel.org
Subject: [PATCH] drm/radeon/dpm: Replace one-element array and use struct_size() helper
Date: Thu, 21 May 2020 20:25:47 -0500 [thread overview]
Message-ID: <20200522012547.GA6783@embeddedor> (raw)
The current codebase makes use of one-element arrays in the following
form:
struct something {
int length;
u8 data[1];
};
struct something *instance;
instance = kmalloc(sizeof(*instance) + size, GFP_KERNEL);
instance->length = size;
memcpy(instance->data, source, size);
but the preferred mechanism to declare variable-length types such as
these ones is a flexible array member[1][2], introduced in C99:
struct foo {
int stuff;
struct boo array[];
};
By making use of the mechanism above, we will get a compiler warning
in case the flexible array does not occur last in the structure, which
will help us prevent some kind of undefined behavior bugs from being
inadvertently introduced[3] to the codebase from now on. So, replace
the one-element array with a flexible-array member.
Also, make use of the new struct_size() helper to properly calculate the
size of struct NISLANDS_SMC_SWSTATE.
This issue was found with the help of Coccinelle and, audited and fixed
_manually_.
[1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
[2] https://github.com/KSPP/linux/issues/21
[3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour")
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
---
drivers/gpu/drm/amd/amdgpu/si_dpm.h | 2 +-
drivers/gpu/drm/radeon/ni_dpm.c | 7 ++++---
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/si_dpm.h b/drivers/gpu/drm/amd/amdgpu/si_dpm.h
index 6b7d292b919f3..bc0be6818e218 100644
--- a/drivers/gpu/drm/amd/amdgpu/si_dpm.h
+++ b/drivers/gpu/drm/amd/amdgpu/si_dpm.h
@@ -781,7 +781,7 @@ struct NISLANDS_SMC_SWSTATE
uint8_t levelCount;
uint8_t padding2;
uint8_t padding3;
- NISLANDS_SMC_HW_PERFORMANCE_LEVEL levels[1];
+ NISLANDS_SMC_HW_PERFORMANCE_LEVEL levels[];
};
typedef struct NISLANDS_SMC_SWSTATE NISLANDS_SMC_SWSTATE;
diff --git a/drivers/gpu/drm/radeon/ni_dpm.c b/drivers/gpu/drm/radeon/ni_dpm.c
index b57c37ddd164c..7d81dde509dc9 100644
--- a/drivers/gpu/drm/radeon/ni_dpm.c
+++ b/drivers/gpu/drm/radeon/ni_dpm.c
@@ -2685,11 +2685,12 @@ static int ni_upload_sw_state(struct radeon_device *rdev,
struct rv7xx_power_info *pi = rv770_get_pi(rdev);
u16 address = pi->state_table_start +
offsetof(NISLANDS_SMC_STATETABLE, driverState);
- u16 state_size = sizeof(NISLANDS_SMC_SWSTATE) +
- ((NISLANDS_MAX_SMC_PERFORMANCE_LEVELS_PER_SWSTATE - 1) * sizeof(NISLANDS_SMC_HW_PERFORMANCE_LEVEL));
+ NISLANDS_SMC_SWSTATE *smc_state;
+ u16 state_size = struct_size(smc_state, levels,
+ NISLANDS_MAX_SMC_PERFORMANCE_LEVELS_PER_SWSTATE);
int ret;
- NISLANDS_SMC_SWSTATE *smc_state = kzalloc(state_size, GFP_KERNEL);
+ smc_state = kzalloc(state_size, GFP_KERNEL);
if (smc_state == NULL)
return -ENOMEM;
--
2.26.2
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
WARNING: multiple messages have this Message-ID (diff)
From: "Gustavo A. R. Silva" <gustavoars@kernel.org>
To: "Alex Deucher" <alexander.deucher@amd.com>,
"Christian König" <christian.koenig@amd.com>,
"David Airlie" <airlied@linux.ie>,
"Daniel Vetter" <daniel@ffwll.ch>
Cc: amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
linux-kernel@vger.kernel.org,
"Gustavo A. R. Silva" <gustavo@embeddedor.com>
Subject: [PATCH] drm/radeon/dpm: Replace one-element array and use struct_size() helper
Date: Thu, 21 May 2020 20:25:47 -0500 [thread overview]
Message-ID: <20200522012547.GA6783@embeddedor> (raw)
The current codebase makes use of one-element arrays in the following
form:
struct something {
int length;
u8 data[1];
};
struct something *instance;
instance = kmalloc(sizeof(*instance) + size, GFP_KERNEL);
instance->length = size;
memcpy(instance->data, source, size);
but the preferred mechanism to declare variable-length types such as
these ones is a flexible array member[1][2], introduced in C99:
struct foo {
int stuff;
struct boo array[];
};
By making use of the mechanism above, we will get a compiler warning
in case the flexible array does not occur last in the structure, which
will help us prevent some kind of undefined behavior bugs from being
inadvertently introduced[3] to the codebase from now on. So, replace
the one-element array with a flexible-array member.
Also, make use of the new struct_size() helper to properly calculate the
size of struct NISLANDS_SMC_SWSTATE.
This issue was found with the help of Coccinelle and, audited and fixed
_manually_.
[1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
[2] https://github.com/KSPP/linux/issues/21
[3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour")
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
---
drivers/gpu/drm/amd/amdgpu/si_dpm.h | 2 +-
drivers/gpu/drm/radeon/ni_dpm.c | 7 ++++---
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/si_dpm.h b/drivers/gpu/drm/amd/amdgpu/si_dpm.h
index 6b7d292b919f3..bc0be6818e218 100644
--- a/drivers/gpu/drm/amd/amdgpu/si_dpm.h
+++ b/drivers/gpu/drm/amd/amdgpu/si_dpm.h
@@ -781,7 +781,7 @@ struct NISLANDS_SMC_SWSTATE
uint8_t levelCount;
uint8_t padding2;
uint8_t padding3;
- NISLANDS_SMC_HW_PERFORMANCE_LEVEL levels[1];
+ NISLANDS_SMC_HW_PERFORMANCE_LEVEL levels[];
};
typedef struct NISLANDS_SMC_SWSTATE NISLANDS_SMC_SWSTATE;
diff --git a/drivers/gpu/drm/radeon/ni_dpm.c b/drivers/gpu/drm/radeon/ni_dpm.c
index b57c37ddd164c..7d81dde509dc9 100644
--- a/drivers/gpu/drm/radeon/ni_dpm.c
+++ b/drivers/gpu/drm/radeon/ni_dpm.c
@@ -2685,11 +2685,12 @@ static int ni_upload_sw_state(struct radeon_device *rdev,
struct rv7xx_power_info *pi = rv770_get_pi(rdev);
u16 address = pi->state_table_start +
offsetof(NISLANDS_SMC_STATETABLE, driverState);
- u16 state_size = sizeof(NISLANDS_SMC_SWSTATE) +
- ((NISLANDS_MAX_SMC_PERFORMANCE_LEVELS_PER_SWSTATE - 1) * sizeof(NISLANDS_SMC_HW_PERFORMANCE_LEVEL));
+ NISLANDS_SMC_SWSTATE *smc_state;
+ u16 state_size = struct_size(smc_state, levels,
+ NISLANDS_MAX_SMC_PERFORMANCE_LEVELS_PER_SWSTATE);
int ret;
- NISLANDS_SMC_SWSTATE *smc_state = kzalloc(state_size, GFP_KERNEL);
+ smc_state = kzalloc(state_size, GFP_KERNEL);
if (smc_state == NULL)
return -ENOMEM;
--
2.26.2
next reply other threads:[~2020-05-22 1:21 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-22 1:25 Gustavo A. R. Silva [this message]
2020-05-22 1:25 ` [PATCH] drm/radeon/dpm: Replace one-element array and use struct_size() helper Gustavo A. R. Silva
2020-05-22 1:25 ` Gustavo A. R. Silva
2020-05-22 7:00 ` Christian König
2020-05-22 7:00 ` Christian König
2020-05-22 7:00 ` Christian König
2020-05-22 17:27 ` Gustavo A. R. Silva
2020-05-22 17:27 ` Gustavo A. R. Silva
2020-05-22 17:27 ` Gustavo A. R. Silva
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200522012547.GA6783@embeddedor \
--to=gustavoars@kernel.org \
--cc=airlied@linux.ie \
--cc=alexander.deucher@amd.com \
--cc=amd-gfx@lists.freedesktop.org \
--cc=christian.koenig@amd.com \
--cc=daniel@ffwll.ch \
--cc=dri-devel@lists.freedesktop.org \
--cc=gustavo@embeddedor.com \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.