From 78496db0128bd50281e1318602f64ed9509d4b6a Mon Sep 17 00:00:00 2001 From: Alex Elder Date: Mon, 17 Nov 2014 08:08:39 -0600 Subject: [PATCH] greybus: clean up gb_connection_operation_recv() This patch does some cleanup of gb_connection_operation_recv(). - Improve the header comments - Verify message is big enough for header before interpreting beginning of the message as a header - Verify at buffer creation time rather than receive time that no operation buffer is bigger than the maximum allowed. We can then compare the incoming data size against the buffer. - When a response message arrives, record its status in the operation result, not in the buffer status. - Record a buffer overflow as an operation error. Signed-off-by: Alex Elder Signed-off-by: Greg Kroah-Hartman --- drivers/staging/greybus/operation.c | 25 +++++++++++++++++++------ drivers/staging/greybus/operation.h | 1 + 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/drivers/staging/greybus/operation.c b/drivers/staging/greybus/operation.c index f1c7dcf..bc68a5f 100644 --- a/drivers/staging/greybus/operation.c +++ b/drivers/staging/greybus/operation.c @@ -204,6 +204,9 @@ static struct gbuf *gb_operation_gbuf_create(struct gb_operation *operation, struct gbuf *gbuf; gfp_t gfp_flags = data_out ? GFP_KERNEL : GFP_ATOMIC; + if (size > GB_OPERATION_MESSAGE_SIZE_MAX) + return NULL; /* Message too big */ + size += sizeof(*header); gbuf = greybus_alloc_gbuf(operation, size, data_out, gfp_flags); if (!gbuf) @@ -355,9 +358,18 @@ int gb_operation_response_send(struct gb_operation *operation) } /* - * Handle data arriving on a connection. This is called in - * interrupt context, so just copy the incoming data into a buffer - * and do remaining handling via a work queue. + * Handle data arriving on a connection. As soon as we return, the + * incoming data buffer will be reused, so we need to copy the data + * into one of our own operation message buffers. + * + * If the incoming data is an operation response message, look up + * the operation and copy the incoming data into its response + * buffer. Otherwise allocate a new operation and copy the incoming + * data into its request buffer. + * + * This is called in interrupt context, so just copy the incoming + * data into the buffer and do remaining handling via a work queue. + * */ void gb_connection_operation_recv(struct gb_connection *connection, void *data, size_t size) @@ -370,8 +382,8 @@ void gb_connection_operation_recv(struct gb_connection *connection, if (connection->state != GB_CONNECTION_STATE_ENABLED) return; - if (size > GB_OPERATION_MESSAGE_SIZE_MAX) { - gb_connection_err(connection, "message too big"); + if (size < sizeof(*header)) { + gb_connection_err(connection, "message too small"); return; } @@ -388,11 +400,12 @@ void gb_connection_operation_recv(struct gb_connection *connection, cancel_delayed_work(&operation->timeout_work); gb_pending_operation_remove(operation); gbuf = operation->response; - gbuf->status = GB_OP_SUCCESS; /* If we got here we're good */ if (size > gbuf->transfer_buffer_length) { + operation->result = GB_OP_OVERFLOW; gb_connection_err(connection, "recv buffer too small"); return; } + operation->result = GB_OP_SUCCESS; } else { WARN_ON(msg_size != size); operation = gb_operation_create(connection, header->type, diff --git a/drivers/staging/greybus/operation.h b/drivers/staging/greybus/operation.h index 4913f72..f30b162 100644 --- a/drivers/staging/greybus/operation.h +++ b/drivers/staging/greybus/operation.h @@ -18,6 +18,7 @@ enum gb_operation_status { GB_OP_INTERRUPTED = 3, GB_OP_RETRY = 4, GB_OP_PROTOCOL_BAD = 5, + GB_OP_OVERFLOW = 6, GB_OP_TIMEOUT = 0xff, }; -- 2.7.4