summaryrefslogtreecommitdiff
path: root/drivers/platform/surface
diff options
context:
space:
mode:
authorMaximilian Luz <luzmaximilian@gmail.com>2021-01-11 16:48:51 +0100
committerHans de Goede <hdegoede@redhat.com>2021-01-13 10:30:21 +0100
commite94a26504f41b2ad33ea1473d32506b01bf6693a (patch)
treee371e98e6b994e33c6e39d798b4f3622243ea058 /drivers/platform/surface
parenta403c1dfcf9fb73a4a362f14ca1c1f04662c49e0 (diff)
platform/surface: aggregator_cdev: Add comments regarding unchecked allocation size
CI static analysis complains about the allocation size in payload and response buffers being unchecked. In general, these allocations should be safe as the user-input is u16 and thus limited to U16_MAX, which is only slightly larger than the theoretical maximum imposed by the underlying SSH protocol. All bounds on these values required by the underlying protocol are enforced in ssam_request_sync() (or rather the functions called by it), thus bounds here are only relevant for allocation. Add comments explaining that this should be safe. Reported-by: Colin Ian King <colin.king@canonical.com> Fixes: 178f6ab77e61 ("platform/surface: Add Surface Aggregator user-space interface") Addresses-Coverity: ("Untrusted allocation size") Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com> Link: https://lore.kernel.org/r/20210111154851.325404-3-luzmaximilian@gmail.com Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Diffstat (limited to 'drivers/platform/surface')
-rw-r--r--drivers/platform/surface/surface_aggregator_cdev.c19
1 files changed, 19 insertions, 0 deletions
diff --git a/drivers/platform/surface/surface_aggregator_cdev.c b/drivers/platform/surface/surface_aggregator_cdev.c
index 979340cdd9de..79e28fab7e40 100644
--- a/drivers/platform/surface/surface_aggregator_cdev.c
+++ b/drivers/platform/surface/surface_aggregator_cdev.c
@@ -106,6 +106,15 @@ static long ssam_cdev_request(struct ssam_cdev *cdev, unsigned long arg)
goto out;
}
+ /*
+ * Note: spec.length is limited to U16_MAX bytes via struct
+ * ssam_cdev_request. This is slightly larger than the
+ * theoretical maximum (SSH_COMMAND_MAX_PAYLOAD_SIZE) of the
+ * underlying protocol (note that nothing remotely this size
+ * should ever be allocated in any normal case). This size is
+ * validated later in ssam_request_sync(), for allocation the
+ * bound imposed by u16 should be enough.
+ */
spec.payload = kzalloc(spec.length, GFP_KERNEL);
if (!spec.payload) {
ret = -ENOMEM;
@@ -125,6 +134,16 @@ static long ssam_cdev_request(struct ssam_cdev *cdev, unsigned long arg)
goto out;
}
+ /*
+ * Note: rsp.capacity is limited to U16_MAX bytes via struct
+ * ssam_cdev_request. This is slightly larger than the
+ * theoretical maximum (SSH_COMMAND_MAX_PAYLOAD_SIZE) of the
+ * underlying protocol (note that nothing remotely this size
+ * should ever be allocated in any normal case). In later use,
+ * this capacity does not have to be strictly bounded, as it
+ * is only used as an output buffer to be written to. For
+ * allocation the bound imposed by u16 should be enough.
+ */
rsp.pointer = kzalloc(rsp.capacity, GFP_KERNEL);
if (!rsp.pointer) {
ret = -ENOMEM;