Currently in amd_pmc_stb_debugfs_open_v2() the buffer size is assumed to
be fixed and a second call to amd_pmc_stb_debugfs_open_v2() may race with
a process holding open another fd. This could change "fsize" to a
bigger size causing an out of bounds read.
Instead create a struct with a flexarray to solve this.
Suggested-by: Hans de Goede <hdegoede@xxxxxxxxxx>
Reviewed-by: Hans de Goede <hdegoede@xxxxxxxxxx>
Signed-off-by: Sanket Goswami <Sanket.Goswami@xxxxxxx>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@xxxxxxx>
---
v7->v8:
- use min()
v6->v7:
- No change
v6:
- Handle release buffer case as per Hans remarks
- based on review-ilpo branch
v5:
- new patch based on comments in v4 from Hans.
- based on review-ilpo branch
drivers/platform/x86/amd/pmc/pmc.c | 32 ++++++++++++++++++------------
1 file changed, 19 insertions(+), 13 deletions(-)
diff --git a/drivers/platform/x86/amd/pmc/pmc.c b/drivers/platform/x86/amd/pmc/pmc.c
index c92dd5077a16..609bad470fe3 100644
--- a/drivers/platform/x86/amd/pmc/pmc.c
+++ b/drivers/platform/x86/amd/pmc/pmc.c
@@ -52,7 +52,7 @@
#define AMD_S2D_REGISTER_ARGUMENT 0xA88
/* STB Spill to DRAM Parameters */
-#define S2D_TELEMETRY_BYTES_MAX 0x100000
+#define S2D_TELEMETRY_BYTES_MAX 0x100000U
#define S2D_TELEMETRY_DRAMBYTES_MAX 0x1000000
/* STB Spill to DRAM Message Definition */
@@ -122,6 +122,11 @@ enum s2d_arg {
S2D_DRAM_SIZE,
};
+struct amd_pmc_stb_v2_data {
+ size_t size;
+ u8 data[] __counted_by(size);
+};
+
struct amd_pmc_bit_map {
const char *name;
u32 bit_mask;
@@ -239,7 +244,8 @@ static const struct file_operations amd_pmc_stb_debugfs_fops = {
static int amd_pmc_stb_debugfs_open_v2(struct inode *inode, struct file *filp)
{
struct amd_pmc_dev *dev = filp->f_inode->i_private;
- u32 *buf, fsize, num_samples, val, stb_rdptr_offset = 0;
+ u32 fsize, num_samples, val, stb_rdptr_offset = 0;
+ struct amd_pmc_stb_v2_data *flex_arr;
int ret;
/* Write dummy postcode while reading the STB buffer */
@@ -247,10 +253,6 @@ static int amd_pmc_stb_debugfs_open_v2(struct inode *inode, struct file *filp)
if (ret)
dev_err(dev->dev, "error writing to STB: %d\n", ret);
- buf = kzalloc(S2D_TELEMETRY_BYTES_MAX, GFP_KERNEL);
- if (!buf)
- return -ENOMEM;
-
/* Spill to DRAM num_samples uses separate SMU message port */
dev->msg_port = 1;
@@ -264,10 +266,16 @@ static int amd_pmc_stb_debugfs_open_v2(struct inode *inode, struct file *filp)
dev->msg_port = 0;
if (ret) {
dev_err(dev->dev, "error: S2D_NUM_SAMPLES not supported : %d\n", ret);
- kfree(buf);
return ret;
}
+ fsize = min(num_samples, S2D_TELEMETRY_BYTES_MAX);
+ flex_arr = kmalloc(struct_size(flex_arr, data, fsize), GFP_KERNEL);
+ if (!flex_arr)
+ return -ENOMEM;
+
+ flex_arr->size = fsize;
+
/* Start capturing data from the last push location */
if (num_samples > S2D_TELEMETRY_BYTES_MAX) {
fsize = S2D_TELEMETRY_BYTES_MAX;
@@ -277,8 +285,8 @@ static int amd_pmc_stb_debugfs_open_v2(struct inode *inode, struct file *filp)
stb_rdptr_offset = 0;
}
- memcpy_fromio(buf, dev->stb_virt_addr + stb_rdptr_offset, fsize);
- filp->private_data = buf;
+ memcpy_fromio(flex_arr->data, dev->stb_virt_addr + stb_rdptr_offset, fsize);
+ filp->private_data = flex_arr;
return 0;
}
@@ -286,11 +294,9 @@ static int amd_pmc_stb_debugfs_open_v2(struct inode *inode, struct file *filp)
static ssize_t amd_pmc_stb_debugfs_read_v2(struct file *filp, char __user *buf, size_t size,
loff_t *pos)
{
- if (!filp->private_data)
- return -EINVAL;
+ struct amd_pmc_stb_v2_data *data = filp->private_data;
- return simple_read_from_buffer(buf, size, pos, filp->private_data,
- S2D_TELEMETRY_BYTES_MAX);
+ return simple_read_from_buffer(buf, size, pos, data->data, data->size);
}
static int amd_pmc_stb_debugfs_release_v2(struct inode *inode, struct file *filp)
--
2.25.1
