greybus: svc: reject invalid state requests

The request sequence for SVC protocol is fixed at least upto SVC_HELLO
request. The first request has to be Protocol Version, followed by
SVC_HELLO. Any other request can follow them, but these two.

Add another field in 'struct gb_svc' that keeps track of current state
of the protocol driver. It tracks only upto SVC_HELLO, as we don't need
to track later ones.

Also add a comment, about the order in which the requests are allowed
and why a race can't happen while accessing 'state'.

This removes the WARN_ON() in gb_svc_hello() as we track state
transition with 'state' field.

This also fixes a crash, when the hotplug request is received before
fully initializing the svc connection. The crash mostly happens while
accessing svc->connection->bundle, which is NULL, but can happen at
other places too, as svc connection isn't fully initialized.

Reported-by: Johan Hovold <johan@hovoldconsulting.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
[johan: add 0x-prefix to warning message ]
Signed-off-by: Johan Hovold <johan@hovoldconsulting.com>
This commit is contained in:
Viresh Kumar 2015-09-03 15:42:22 +05:30 committed by Johan Hovold
parent f66427adfd
commit 3ccb1600ba

View File

@ -15,8 +15,15 @@
#define CPORT_FLAGS_CSD_N (2)
#define CPORT_FLAGS_CSV_N (4)
enum gb_svc_state {
GB_SVC_STATE_RESET,
GB_SVC_STATE_PROTOCOL_VERSION,
GB_SVC_STATE_SVC_HELLO,
};
struct gb_svc {
struct gb_connection *connection;
enum gb_svc_state state;
};
struct svc_hotplug {
@ -226,9 +233,6 @@ static int gb_svc_hello(struct gb_operation *op)
u8 interface_id;
int ret;
/* Hello message should be received only during early bootup */
WARN_ON(hd->initial_svc_connection != connection);
/*
* SVC sends information about the endo and interface-id on the hello
* request, use that to create an endo.
@ -452,11 +456,53 @@ static int gb_svc_intf_reset_recv(struct gb_operation *op)
static int gb_svc_request_recv(u8 type, struct gb_operation *op)
{
struct gb_connection *connection = op->connection;
struct gb_svc *svc = connection->private;
int ret = 0;
/*
* SVC requests need to follow a specific order (at least initially) and
* below code takes care of enforcing that. The expected order is:
* - PROTOCOL_VERSION
* - SVC_HELLO
* - Any other request, but the earlier two.
*
* Incoming requests are guaranteed to be serialized and so we don't
* need to protect 'state' for any races.
*/
switch (type) {
case GB_REQUEST_TYPE_PROTOCOL_VERSION:
return gb_svc_version_request(op);
if (svc->state != GB_SVC_STATE_RESET)
ret = -EINVAL;
break;
case GB_SVC_TYPE_SVC_HELLO:
return gb_svc_hello(op);
if (svc->state != GB_SVC_STATE_PROTOCOL_VERSION)
ret = -EINVAL;
break;
default:
if (svc->state != GB_SVC_STATE_SVC_HELLO)
ret = -EINVAL;
break;
}
if (ret) {
dev_warn(&connection->dev,
"unexpected SVC request 0x%02x received (state %u)\n",
type, svc->state);
return ret;
}
switch (type) {
case GB_REQUEST_TYPE_PROTOCOL_VERSION:
ret = gb_svc_version_request(op);
if (!ret)
svc->state = GB_SVC_STATE_PROTOCOL_VERSION;
return ret;
case GB_SVC_TYPE_SVC_HELLO:
ret = gb_svc_hello(op);
if (!ret)
svc->state = GB_SVC_STATE_SVC_HELLO;
return ret;
case GB_SVC_TYPE_INTF_HOTPLUG:
return gb_svc_intf_hotplug_recv(op);
case GB_SVC_TYPE_INTF_HOT_UNPLUG:
@ -479,6 +525,7 @@ static int gb_svc_connection_init(struct gb_connection *connection)
return -ENOMEM;
connection->hd->svc = svc;
svc->state = GB_SVC_STATE_RESET;
svc->connection = connection;
connection->private = svc;