Hi Dan,
Thanks for the detailed review.
- Fixed the From header so it matches the Signed-off-by. - Dropped the review-style commentary from the commit message. - Switched every site to sizeof(buffer->data) instead of PAGE_SIZE (I'd only done this in one of the four spots in v1 - fixed the other three too). - You're right that none of these can actually overflow today: capabilities tops out around 3 bytes per input byte on a payload that's at most ~1KB, nstreams is on the order of 4, and req_id is just a u32 printed as a string. So no Fixes tag, and I reworded the commit message so it doesn't reference any current or hypothetical overflow - it's a plain style/robustness change now.
v2 below.
Thanks, Vaibhav
---
From: Vaibhav Agarwal contectforbusiness@proton.me Subject: [PATCH v2] staging: greybus: camera: use scnprintf() for debugfs buffers
The debugfs buffers in gb_camera are written with sprintf(). Use scnprintf() with sizeof(buffer->data) instead, which is the usual kernel pattern for writing into a fixed-size buffer and makes the code easier to audit.
No functional change.
Signed-off-by: Vaibhav Agarwal contectforbusiness@proton.me --- Changes in v2: - Fix From header to match Signed-off-by - Drop review-thread commentary from the commit message - Use sizeof(buffer->data) instead of PAGE_SIZE at all four sites (v1 only did this at one of the four) - Reword commit message to not reference any overflow, current or hypothetical - none of the three call sites can overflow today
drivers/staging/greybus/camera.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/greybus/camera.c b/drivers/staging/greybus/camera.c index 62b55bb28..0e9d7642a 100644 --- a/drivers/staging/greybus/camera.c +++ b/drivers/staging/greybus/camera.c @@ -890,7 +890,8 @@ static ssize_t gb_camera_debugfs_capabilities(struct gb_camera *gcam, for (i = 0; i < size; i += 16) { unsigned int nbytes = min_t(unsigned int, size - i, 16);
- buffer->length += sprintf(buffer->data + buffer->length, + buffer->length += scnprintf(buffer->data + buffer->length, + sizeof(buffer->data) - buffer->length, "%*ph\n", nbytes, caps + i); }
@@ -973,12 +974,13 @@ static ssize_t gb_camera_debugfs_configure_streams(struct gb_camera *gcam, if (ret < 0) goto done;
- buffer->length = sprintf(buffer->data, "%u;%u;", nstreams, flags); + buffer->length = scnprintf(buffer->data, sizeof(buffer->data), "%u;%u;", nstreams, flags);
for (i = 0; i < nstreams; ++i) { struct gb_camera_stream_config *stream = &streams[i];
- buffer->length += sprintf(buffer->data + buffer->length, + buffer->length += scnprintf(buffer->data + buffer->length, + sizeof(buffer->data) - buffer->length, "%u;%u;%u;%u;%u;%u;%u;", stream->width, stream->height, stream->format, stream->vc, @@ -1046,7 +1048,7 @@ static ssize_t gb_camera_debugfs_flush(struct gb_camera *gcam, if (ret < 0) return ret;
- buffer->length = sprintf(buffer->data, "%u", req_id); + buffer->length = scnprintf(buffer->data, sizeof(buffer->data), "%u", req_id);
return len; }