greybus: manifest: reserve control connection cport/bundle ids
5ae6906e ('interface: Get manifest using Control protocol') in gb_create_control_connection introduces the concept that the Control Protocol is at cport_id 2 and bundle_id 0. Currently the manifest parsing code does not enforce that concept and as a result it is possible for a manifest to declare the Control Protocol at a different address. Based on that change 6a6945c9684e ('greybus-spec/control: Formally define Control Protocol reserved addresses') makes the above coding convention a formal requirement of the greybus specification. This patch implements the change introduced in the specification @ 6a6945c9684e. This patch will reject a manifest if it doesn't match the critiera laid down in the spec. This patch makes three changes: - Changes gb_manifest_parse_cports so that only GB_CONTROL_CPORT_ID may have a protocol_id of GREYBUS_PROTOCOL_CONTROL, otherwise the manifest will be rejected. - Changes gb_manifest_parse_bundles so that only GB_CONTROL_BUNDLE_ID may have a class of GREYBUS_CLASS_CONTROL, otherwise the manifest will be rejected. - gb_connection_exit() and gb_connection_destroy() are removed from gb_manifest_parse_cports on error - since gb_manifest_parse_bundles() already has a call into gb_bundle_destroy() which will again call gb_connection_exit() and gb_connection_destroy() leading to an oops. Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org> Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
This commit is contained in:
parent
18d777cd66
commit
b38fe3472d
|
@ -201,18 +201,16 @@ static char *gb_string_get(struct gb_interface *intf, u8 string_id)
|
|||
static u32 gb_manifest_parse_cports(struct gb_bundle *bundle)
|
||||
{
|
||||
struct gb_interface *intf = bundle->intf;
|
||||
struct gb_connection *connection;
|
||||
struct gb_connection *connection_next;
|
||||
struct manifest_desc *desc;
|
||||
struct manifest_desc *next;
|
||||
u8 bundle_id = bundle->id;
|
||||
u8 protocol_id;
|
||||
u16 cport_id;
|
||||
u32 count = 0;
|
||||
|
||||
/* Set up all cport descriptors associated with this bundle */
|
||||
list_for_each_entry_safe(desc, next, &intf->manifest_descs, links) {
|
||||
struct greybus_descriptor_cport *desc_cport;
|
||||
u8 protocol_id;
|
||||
u16 cport_id;
|
||||
|
||||
if (desc->type != GREYBUS_TYPE_CPORT)
|
||||
continue;
|
||||
|
@ -223,25 +221,26 @@ static u32 gb_manifest_parse_cports(struct gb_bundle *bundle)
|
|||
|
||||
cport_id = le16_to_cpu(desc_cport->id);
|
||||
if (cport_id > CPORT_ID_MAX)
|
||||
goto cleanup;
|
||||
goto exit;
|
||||
|
||||
/* Found one. Set up its function structure */
|
||||
protocol_id = desc_cport->protocol_id;
|
||||
|
||||
/* Don't recreate connection for control cport */
|
||||
/* Validate declarations of the control protocol CPort */
|
||||
if (cport_id == GB_CONTROL_CPORT_ID) {
|
||||
/* This should have protocol set to control protocol*/
|
||||
WARN_ON(protocol_id != GREYBUS_PROTOCOL_CONTROL);
|
||||
|
||||
if (protocol_id != GREYBUS_PROTOCOL_CONTROL)
|
||||
goto print_error_exit;
|
||||
/* Don't recreate connection for control cport */
|
||||
goto release_descriptor;
|
||||
}
|
||||
|
||||
/* Nothing else should have its protocol as control protocol */
|
||||
if (WARN_ON(protocol_id == GREYBUS_PROTOCOL_CONTROL))
|
||||
goto release_descriptor;
|
||||
if (protocol_id == GREYBUS_PROTOCOL_CONTROL) {
|
||||
goto print_error_exit;
|
||||
}
|
||||
|
||||
if (!gb_connection_create(bundle, cport_id, protocol_id))
|
||||
goto cleanup;
|
||||
goto exit;
|
||||
|
||||
release_descriptor:
|
||||
count++;
|
||||
|
@ -251,14 +250,14 @@ release_descriptor:
|
|||
}
|
||||
|
||||
return count;
|
||||
cleanup:
|
||||
/* An error occurred; undo any changes we've made */
|
||||
list_for_each_entry_safe(connection, connection_next,
|
||||
&bundle->connections, bundle_links) {
|
||||
gb_connection_exit(connection);
|
||||
gb_connection_destroy(connection);
|
||||
count--;
|
||||
}
|
||||
print_error_exit:
|
||||
/* A control protocol parse error was encountered */
|
||||
dev_err(&bundle->dev,
|
||||
"cport_id, protocol_id 0x%04hx,0x%04hx want 0x%04hx,0x%04hx\n",
|
||||
cport_id, protocol_id, GB_CONTROL_CPORT_ID,
|
||||
GREYBUS_PROTOCOL_CONTROL);
|
||||
exit:
|
||||
|
||||
return 0; /* Error; count should also be 0 */
|
||||
}
|
||||
|
||||
|
@ -287,15 +286,24 @@ static u32 gb_manifest_parse_bundles(struct gb_interface *intf)
|
|||
/* Don't recreate bundle for control cport */
|
||||
if (desc_bundle->id == GB_CONTROL_BUNDLE_ID) {
|
||||
/* This should have class set to control class */
|
||||
WARN_ON(desc_bundle->class != GREYBUS_CLASS_CONTROL);
|
||||
if (desc_bundle->class != GREYBUS_CLASS_CONTROL) {
|
||||
dev_err(&intf->dev,
|
||||
"bad class 0x%02x for control bundle\n",
|
||||
desc_bundle->class);
|
||||
goto cleanup;
|
||||
}
|
||||
|
||||
bundle = intf->control->connection->bundle;
|
||||
goto parse_cports;
|
||||
}
|
||||
|
||||
/* Nothing else should have its class set to control class */
|
||||
if (WARN_ON(desc_bundle->class == GREYBUS_CLASS_CONTROL))
|
||||
goto release_descriptor;
|
||||
if (desc_bundle->class == GREYBUS_CLASS_CONTROL) {
|
||||
dev_err(&intf->dev,
|
||||
"bundle 0x%02x cannot use control class\n",
|
||||
desc_bundle->id);
|
||||
goto cleanup;
|
||||
}
|
||||
|
||||
bundle = gb_bundle_create(intf, desc_bundle->id,
|
||||
desc_bundle->class);
|
||||
|
@ -307,11 +315,9 @@ parse_cports:
|
|||
if (!gb_manifest_parse_cports(bundle))
|
||||
goto cleanup;
|
||||
|
||||
release_descriptor:
|
||||
count++;
|
||||
|
||||
/* Done with this bundle descriptor */
|
||||
release_manifest_descriptor(desc);
|
||||
count++;
|
||||
}
|
||||
|
||||
return count;
|
||||
|
|
Loading…
Reference in New Issue