Re: [PATCH V1] accel/amdxdna: Adjust size for copy_to_user()
From: Mario Limonciello
Date: Thu Apr 02 2026 - 13:26:35 EST
On 4/2/26 12:14 PM, Lizhi Hou wrote:
The amount of data returned to user space should be limited by the buffer
size provided by the application. If the buffer is smaller than the data
size, return only the portion that fits instead of failing.
Prolly should have a fixes tag for this I'd think.
Signed-off-by: Lizhi Hou <lizhi.hou@xxxxxxx>
---
drivers/accel/amdxdna/aie2_error.c | 5 ++-
drivers/accel/amdxdna/aie2_message.c | 20 ++++++----
drivers/accel/amdxdna/aie2_pci.c | 59 ++++++++++++++++------------
3 files changed, 50 insertions(+), 34 deletions(-)
diff --git a/drivers/accel/amdxdna/aie2_error.c b/drivers/accel/amdxdna/aie2_error.c
index 9d20e956c020..70007b4363cd 100644
--- a/drivers/accel/amdxdna/aie2_error.c
+++ b/drivers/accel/amdxdna/aie2_error.c
@@ -406,8 +406,11 @@ int aie2_get_array_async_error(struct amdxdna_dev_hdl *ndev, struct amdxdna_drm_
drm_WARN_ON(&xdna->ddev, !mutex_is_locked(&xdna->dev_lock));
+ if (!args->num_element)
+ return -EINVAL;
+
args->num_element = 1;
- args->element_size = sizeof(ndev->last_async_err);
+ args->element_size = min(args->element_size, sizeof(ndev->last_async_err));
if (copy_to_user(u64_to_user_ptr(args->buffer),
&ndev->last_async_err, args->element_size))
return -EFAULT;
diff --git a/drivers/accel/amdxdna/aie2_message.c b/drivers/accel/amdxdna/aie2_message.c
index e5e7da7a8f40..e52dc7ea9fc7 100644
--- a/drivers/accel/amdxdna/aie2_message.c
+++ b/drivers/accel/amdxdna/aie2_message.c
@@ -369,12 +369,13 @@ int aie2_query_status(struct amdxdna_dev_hdl *ndev, char __user *buf,
{
DECLARE_AIE_MSG(aie_column_info, MSG_OP_QUERY_COL_STATUS);
struct amdxdna_dev *xdna = ndev->aie.xdna;
- u32 buf_sz = size, aie_bitmap = 0;
+ u32 buf_sz, aie_bitmap = 0;
struct amdxdna_client *client;
dma_addr_t dma_addr;
u8 *buff_addr;
int ret;
+ buf_sz = ndev->metadata.cols * ndev->metadata.size;
buff_addr = aie2_alloc_msg_buffer(ndev, &buf_sz, &dma_addr);
if (IS_ERR(buff_addr))
return PTR_ERR(buff_addr);
@@ -398,13 +399,14 @@ int aie2_query_status(struct amdxdna_dev_hdl *ndev, char __user *buf,
XDNA_DBG(xdna, "Query NPU status completed");
- if (size < resp.size) {
+ if (buf_sz < resp.size) {
ret = -EINVAL;
- XDNA_ERR(xdna, "Bad buffer size. Available: %u. Needs: %u", size, resp.size);
+ XDNA_ERR(xdna, "Bad buffer size. Available: %u. Needs: %u", buf_sz, resp.size);
goto fail;
}
- if (copy_to_user(buf, buff_addr, resp.size)) {
+ size = min(size, resp.size);
+ if (copy_to_user(buf, buff_addr, size)) {
ret = -EFAULT;
XDNA_ERR(xdna, "Failed to copy NPU status to user space");
goto fail;
@@ -424,13 +426,14 @@ int aie2_query_telemetry(struct amdxdna_dev_hdl *ndev,
DECLARE_AIE_MSG(get_telemetry, MSG_OP_GET_TELEMETRY);
struct amdxdna_dev *xdna = ndev->aie.xdna;
dma_addr_t dma_addr;
- u32 buf_sz = size;
+ u32 buf_sz;
u8 *addr;
int ret;
if (header->type >= MAX_TELEMETRY_TYPE)
return -EINVAL;
+ buf_sz = min(size, SZ_4M);
addr = aie2_alloc_msg_buffer(ndev, &buf_sz, &dma_addr);
if (IS_ERR(addr))
return PTR_ERR(addr);
@@ -446,13 +449,14 @@ int aie2_query_telemetry(struct amdxdna_dev_hdl *ndev,
goto free_buf;
}
- if (size < resp.size) {
+ if (buf_sz < resp.size) {
ret = -EINVAL;
- XDNA_ERR(xdna, "Bad buffer size. Available: %u. Needs: %u", size, resp.size);
+ XDNA_ERR(xdna, "Bad buffer size. Available: %u. Needs: %u", buf_sz, resp.size);
goto free_buf;
}
- if (copy_to_user(buf, addr, resp.size)) {
+ size = min(size, resp.size);
+ if (copy_to_user(buf, addr, size)) {
ret = -EFAULT;
XDNA_ERR(xdna, "Failed to copy telemetry to user space");
goto free_buf;
diff --git a/drivers/accel/amdxdna/aie2_pci.c b/drivers/accel/amdxdna/aie2_pci.c
index 164e188ba501..041cbc8cd7e5 100644
--- a/drivers/accel/amdxdna/aie2_pci.c
+++ b/drivers/accel/amdxdna/aie2_pci.c
@@ -620,23 +620,19 @@ static void aie2_fini(struct amdxdna_dev *xdna)
static int aie2_get_aie_status(struct amdxdna_client *client,
struct amdxdna_drm_get_info *args)
{
- struct amdxdna_drm_query_aie_status status;
+ struct amdxdna_drm_query_aie_status status = {};
struct amdxdna_dev *xdna = client->xdna;
struct amdxdna_dev_hdl *ndev;
+ u32 buf_sz;
int ret;
ndev = xdna->dev_handle;
- if (copy_from_user(&status, u64_to_user_ptr(args->buffer), sizeof(status))) {
+ buf_sz = min(args->buffer_size, sizeof(status));
+ if (copy_from_user(&status, u64_to_user_ptr(args->buffer), buf_sz)) {
XDNA_ERR(xdna, "Failed to copy AIE request into kernel");
return -EFAULT;
}
- if (ndev->metadata.cols * ndev->metadata.size < status.buffer_size) {
- XDNA_ERR(xdna, "Invalid buffer size. Given Size: %u. Need Size: %u.",
- status.buffer_size, ndev->metadata.cols * ndev->metadata.size);
- return -EINVAL;
- }
-
ret = aie2_query_status(ndev, u64_to_user_ptr(status.buffer),
status.buffer_size, &status.cols_filled);
if (ret) {
@@ -644,7 +640,7 @@ static int aie2_get_aie_status(struct amdxdna_client *client,
return ret;
}
- if (copy_to_user(u64_to_user_ptr(args->buffer), &status, sizeof(status))) {
+ if (copy_to_user(u64_to_user_ptr(args->buffer), &status, buf_sz)) {
XDNA_ERR(xdna, "Failed to copy AIE request info to user space");
return -EFAULT;
}
@@ -659,6 +655,7 @@ static int aie2_get_aie_metadata(struct amdxdna_client *client,
struct amdxdna_dev *xdna = client->xdna;
struct amdxdna_dev_hdl *ndev;
int ret = 0;
+ u32 buf_sz;
ndev = xdna->dev_handle;
meta = kzalloc_obj(*meta);
@@ -690,7 +687,8 @@ static int aie2_get_aie_metadata(struct amdxdna_client *client,
meta->shim.lock_count = ndev->metadata.shim.lock_count;
meta->shim.event_reg_count = ndev->metadata.shim.event_reg_count;
- if (copy_to_user(u64_to_user_ptr(args->buffer), meta, sizeof(*meta)))
+ buf_sz = min(args->buffer_size, sizeof(*meta));
+ if (copy_to_user(u64_to_user_ptr(args->buffer), meta, buf_sz))
ret = -EFAULT;
kfree(meta);
@@ -703,12 +701,14 @@ static int aie2_get_aie_version(struct amdxdna_client *client,
struct amdxdna_drm_query_aie_version version;
struct amdxdna_dev *xdna = client->xdna;
struct amdxdna_dev_hdl *ndev;
+ u32 buf_sz;
ndev = xdna->dev_handle;
version.major = ndev->version.major;
version.minor = ndev->version.minor;
- if (copy_to_user(u64_to_user_ptr(args->buffer), &version, sizeof(version)))
+ buf_sz = min(args->buffer_size, sizeof(version));
+ if (copy_to_user(u64_to_user_ptr(args->buffer), &version, buf_sz))
return -EFAULT;
return 0;
@@ -719,13 +719,15 @@ static int aie2_get_firmware_version(struct amdxdna_client *client,
{
struct amdxdna_drm_query_firmware_version version;
struct amdxdna_dev *xdna = client->xdna;
+ u32 buf_sz;
version.major = xdna->fw_ver.major;
version.minor = xdna->fw_ver.minor;
version.patch = xdna->fw_ver.sub;
version.build = xdna->fw_ver.build;
- if (copy_to_user(u64_to_user_ptr(args->buffer), &version, sizeof(version)))
+ buf_sz = min(args->buffer_size, sizeof(version));
+ if (copy_to_user(u64_to_user_ptr(args->buffer), &version, buf_sz))
return -EFAULT;
return 0;
@@ -737,11 +739,13 @@ static int aie2_get_power_mode(struct amdxdna_client *client,
struct amdxdna_drm_get_power_mode mode = {};
struct amdxdna_dev *xdna = client->xdna;
struct amdxdna_dev_hdl *ndev;
+ u32 buf_sz;
ndev = xdna->dev_handle;
mode.power_mode = ndev->pw_mode;
- if (copy_to_user(u64_to_user_ptr(args->buffer), &mode, sizeof(mode)))
+ buf_sz = min(args->buffer_size, sizeof(mode));
+ if (copy_to_user(u64_to_user_ptr(args->buffer), &mode, buf_sz))
return -EFAULT;
return 0;
@@ -754,6 +758,7 @@ static int aie2_get_clock_metadata(struct amdxdna_client *client,
struct amdxdna_dev *xdna = client->xdna;
struct amdxdna_dev_hdl *ndev;
int ret = 0;
+ u32 buf_sz;
ndev = xdna->dev_handle;
clock = kzalloc_obj(*clock);
@@ -766,7 +771,8 @@ static int aie2_get_clock_metadata(struct amdxdna_client *client,
snprintf(clock->h_clock.name, sizeof(clock->h_clock.name), "H Clock");
clock->h_clock.freq_mhz = ndev->hclk_freq;
- if (copy_to_user(u64_to_user_ptr(args->buffer), clock, sizeof(*clock)))
+ buf_sz = min(args->buffer_size, sizeof(*clock));
+ if (copy_to_user(u64_to_user_ptr(args->buffer), clock, buf_sz))
ret = -EFAULT;
kfree(clock);
@@ -792,12 +798,14 @@ static int aie2_get_sensors(struct amdxdna_client *client,
scnprintf(sensor.label, sizeof(sensor.label), "Total Power");
scnprintf(sensor.units, sizeof(sensor.units), "mW");
+ if (args->buffer_size < sizeof(sensor))
+ goto out;
+
if (copy_to_user(u64_to_user_ptr(args->buffer), &sensor, sizeof(sensor)))
return -EFAULT;
+ args->buffer_size -= sizeof(sensor);
sensors_count++;
- if (args->buffer_size <= sensors_count * sizeof(sensor))
- goto out;
for (i = 0; i < min_t(u32, ndev->total_col, 8); i++) {
memset(&sensor, 0, sizeof(sensor));
@@ -807,13 +815,15 @@ static int aie2_get_sensors(struct amdxdna_client *client,
scnprintf(sensor.label, sizeof(sensor.label), "Column %d Utilization", i);
scnprintf(sensor.units, sizeof(sensor.units), "%%");
+ if (args->buffer_size < sizeof(sensor))
+ goto out;
+
if (copy_to_user(u64_to_user_ptr(args->buffer) + sensors_count * sizeof(sensor),
&sensor, sizeof(sensor)))
return -EFAULT;
+ args->buffer_size -= sizeof(sensor);
sensors_count++;
- if (args->buffer_size <= sensors_count * sizeof(sensor))
- goto out;
}
out:
@@ -909,6 +919,7 @@ static int aie2_query_resource_info(struct amdxdna_client *client,
const struct amdxdna_dev_priv *priv;
struct amdxdna_dev_hdl *ndev;
struct amdxdna_dev *xdna;
+ u32 buf_sz;
xdna = client->xdna;
ndev = xdna->dev_handle;
@@ -920,7 +931,8 @@ static int aie2_query_resource_info(struct amdxdna_client *client,
res_info.npu_tops_curr = ndev->curr_tops;
res_info.npu_task_curr = ndev->hwctx_num;
- if (copy_to_user(u64_to_user_ptr(args->buffer), &res_info, sizeof(res_info)))
+ buf_sz = min(args->buffer_size, sizeof(res_info));
+ if (copy_to_user(u64_to_user_ptr(args->buffer), &res_info, buf_sz))
return -EFAULT;
return 0;
@@ -956,12 +968,7 @@ static int aie2_get_telemetry(struct amdxdna_client *client,
XDNA_ERR(xdna, "Invalid buffer size");
return -EINVAL;
}
-
telemetry_data_sz = args->buffer_size - header_sz;
- if (telemetry_data_sz > SZ_4M) {
- XDNA_ERR(xdna, "Buffer size is too big, %d", telemetry_data_sz);
- return -EINVAL;
- }
header = kzalloc(header_sz, GFP_KERNEL);
if (!header)
@@ -1002,6 +1009,7 @@ static int aie2_get_preempt_state(struct amdxdna_client *client,
struct amdxdna_drm_attribute_state state = {};
struct amdxdna_dev *xdna = client->xdna;
struct amdxdna_dev_hdl *ndev;
+ u32 buf_sz;
ndev = xdna->dev_handle;
if (args->param == DRM_AMDXDNA_GET_FORCE_PREEMPT_STATE)
@@ -1009,7 +1017,8 @@ static int aie2_get_preempt_state(struct amdxdna_client *client,
else if (args->param == DRM_AMDXDNA_GET_FRAME_BOUNDARY_PREEMPT_STATE)
state.state = ndev->frame_boundary_preempt;
- if (copy_to_user(u64_to_user_ptr(args->buffer), &state, sizeof(state)))
+ buf_sz = min(args->buffer_size, sizeof(state));
+ if (copy_to_user(u64_to_user_ptr(args->buffer), &state, buf_sz))
return -EFAULT;
return 0;