[PATCH 2/4] accel/amdxdna: bound the command error payload length

[Eva Crystal]

amdxdna_cmd_set_error() computes the length of the region it scribbles
over from the BO size, without a floor:

	memset(cmd->data, 0xff, abo->mem.size - sizeof(*cmd));
	if (err_data)
		memcpy(cmd->data, err_data, min(size, abo->mem.size - sizeof(*cmd)));

abo->mem.size is a size_t and sizeof(struct amdxdna_cmd) is 4 - the
struct is a u32 header followed by a flexible array. A BO smaller than
four bytes therefore turns both lengths into a value near SIZE_MAX, and
the min() in the memcpy offers no protection because the underflowed
value is the larger operand.

No such BO can reach this function today. Command BOs are created by
drm_gem_shmem_create(), which PAGE_ALIGN()s the size, so mem.size is
either 0 or at least PAGE_SIZE. Zero is reachable - PAGE_ALIGN() wraps
for sizes above ULLONG_MAX - PAGE_SIZE + 1, and nothing rejects it on
the share-BO path - but a zero-sized BO cannot be vmap()ed, because
vmap() refuses a zero-page mapping, so amdxdna_gem_vmap() returns NULL
and the !cmd test above rejects the BO before the subtraction. This is
not a fix for a reachable bug.

That leaves an unguarded size_t subtraction feeding a memset() length,
whose safety depends on a property of a different allocator and on
vmap()'s behaviour for a zero-page request. Compute the length once,
reject a BO too small to hold the header, and use the result for both
the memset() and the memcpy() bound.

Signed-off-by: Eva Crystal <0xiviel@xxxxxxxxx>
---
 drivers/accel/amdxdna/amdxdna_ctx.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/accel/amdxdna/amdxdna_ctx.c b/drivers/accel/amdxdna/amdxdna_ctx.c
index 163b5fc..c24bf1c 100644
--- a/drivers/accel/amdxdna/amdxdna_ctx.c
+++ b/drivers/accel/amdxdna/amdxdna_ctx.c
@@ -152,6 +152,7 @@ int amdxdna_cmd_set_error(struct amdxdna_gem_obj *abo,
 	struct amdxdna_client *client = job->hwctx->client;
 	struct amdxdna_cmd *cmd = amdxdna_gem_vmap(abo);
 	struct amdxdna_cmd_chain *cc = NULL;
+	size_t data_size;
 
 	if (!cmd)
 		return -ENOMEM;
@@ -173,9 +174,16 @@ int amdxdna_cmd_set_error(struct amdxdna_gem_obj *abo,
 			return -ENOMEM;
 	}
 
-	memset(cmd->data, 0xff, abo->mem.size - sizeof(*cmd));
+	if (abo->mem.size < sizeof(*cmd)) {
+		if (cc)
+			amdxdna_gem_put_obj(abo);
+		return -EINVAL;
+	}
+	data_size = abo->mem.size - sizeof(*cmd);
+
+	memset(cmd->data, 0xff, data_size);
 	if (err_data)
-		memcpy(cmd->data, err_data, min(size, abo->mem.size - sizeof(*cmd)));
+		memcpy(cmd->data, err_data, min(size, data_size));
 
 	if (cc)
 		amdxdna_gem_put_obj(abo);
-- 
2.53.0