From b38fe3472d38933b9fb751918a09d65f1483ef8b Mon Sep 17 00:00:00 2001 From: Bryan O'Donoghue Date: Tue, 21 Jul 2015 09:10:27 +0100 Subject: [PATCH] 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 Reviewed-by: Viresh Kumar Signed-off-by: Greg Kroah-Hartman --- drivers/staging/greybus/manifest.c | 58 ++++++++++++++++-------------- 1 file changed, 32 insertions(+), 26 deletions(-) diff --git a/drivers/staging/greybus/manifest.c b/drivers/staging/greybus/manifest.c index bea565f1c1d1..a84c07133575 100644 --- a/drivers/staging/greybus/manifest.c +++ b/drivers/staging/greybus/manifest.c @@ -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;