On 14-Sep-26 8:24 PM, Christian König wrote:
On 9/14/26 16:45, Lazar, Lijo wrote:
On 14-Sep-26 7:30 PM, Christian König wrote:
On 9/14/26 15:56, Dmitriy Chumachenko wrote:
The reg_state readouts return szbuf bytes but never write the pad members
of the headers and of amdgpu_smn_reg_data (and, for pcie, the fields
skipped when the upstream port or AER capability is absent). The sysfs
buffer is a plain kmalloc(), so those bytes leak stale slab data to any
local user through the 0444 reg_state attribute.
Zero the buffer once the size is known.
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Fixes: 081a6eda2b25 ("drm/amdgpu: Read aquavanjaram PCIE register state")
Signed-off-by: Dmitriy Chumachenko <[email protected]>
Good catch, but stuff like that should be handled by using kzalloc() instead of
calling memset it manually.
It's the default sysfs buffer allocation which may not be used entirely by all
drivers.
The concern in this case is that driver is telling it has written x bytes
whereas it could have left some fields unfilled in the buffer. I think the fix
is fine.
Wait a second, is that because of messed up padding in sysfs structures? That
is usually a really big NO-GO for upstreaming.
Structures returned to userspace must be padding free, see the tool pahole for
how to double check that on different architectures.
It's an explicit padding like below for alignment. Those fields also
could go unfilled.
struct amdgpu_reg_state_header {
uint16_t structure_size;
uint8_t format_revision;
uint8_t content_revision;
uint8_t state_type;
uint8_t num_instances;
uint16_t pad;
};
Thanks,
Lijo
Regards,
Christian.
Thanks,
Lijo
Regards,
Christian.
---
drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c
b/drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c
index 72ea37dbfea8..0086de0f685d 100644
--- a/drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c
+++ b/drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c
@@ -610,6 +610,8 @@ static ssize_t aqua_vanjaram_read_pcie_state(struct
amdgpu_device *adev,
if (max_size < szbuf)
return -EOVERFLOW;
+ memset(buf, 0, szbuf);
+
pcie_regs = (struct amdgpu_regs_pcie_v1_0 *)((uint8_t *)buf +
sizeof(*pcie_reg_state));
pcie_regs->inst_header.instance = 0;
@@ -702,6 +704,8 @@ static ssize_t aqua_vanjaram_read_xgmi_state(struct
amdgpu_device *adev,
if (max_size < szbuf)
return -EOVERFLOW;
+ memset(buf, 0, szbuf);
+
p = &xgmi_reg_state->xgmi_state_regs[0];
for_each_inst(i, adev->aid_mask) {
for (j = 0; j < xgmi_inst; ++j) {
@@ -776,6 +780,8 @@ static ssize_t aqua_vanjaram_read_wafl_state(struct
amdgpu_device *adev,
if (max_size < szbuf)
return -EOVERFLOW;
+ memset(buf, 0, szbuf);
+
p = &wafl_reg_state->wafl_state_regs[0];
for_each_inst(i, adev->aid_mask) {
for (j = 0; j < wafl_inst; ++j) {
@@ -902,6 +908,8 @@ static ssize_t aqua_vanjaram_read_usr_state(struct
amdgpu_device *adev,
if (max_size < szbuf)
return -EOVERFLOW;
+ memset(buf, 0, szbuf);
+
p = &usr_reg_state->usr_state_regs[0];
for_each_inst(i, adev->aid_mask) {
usr_regs = (struct amdgpu_regs_usr_v1_0 *)p;
--
2.49.0
