From d0cfd109c7eb5df548dd98bfa7f2dba370c68e1c Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Sun, 21 Sep 2014 19:10:39 -0700 Subject: [PATCH] greybus: ap: validate the rest of the svc message buffer sizes --- drivers/staging/greybus/ap.c | 63 ++++++++++++++++++++++++------- drivers/staging/greybus/core.c | 3 +- drivers/staging/greybus/greybus.h | 3 +- 3 files changed, 54 insertions(+), 15 deletions(-) diff --git a/drivers/staging/greybus/ap.c b/drivers/staging/greybus/ap.c index 267d8b51fd4b..21f2e3327f12 100644 --- a/drivers/staging/greybus/ap.c +++ b/drivers/staging/greybus/ap.c @@ -112,49 +112,86 @@ static void svc_management(struct svc_function_unipro_management *management, } static void svc_hotplug(struct svc_function_hotplug *hotplug, - struct greybus_host_device *hd) + int payload_length, struct greybus_host_device *hd) { u8 module_id = hotplug->module_id; switch (hotplug->hotplug_event) { case SVC_HOTPLUG_EVENT: /* Add a new module to the system */ + if (payload_length < 0x03) { + /* Hotplug message is at lest 3 bytes big */ + dev_err(hd->parent, + "Illegal size of svc hotplug message %d\n", + payload_length); + return; + } dev_dbg(hd->parent, "module id %d added\n", module_id); - gb_add_module(hd, module_id, hotplug->data); + gb_add_module(hd, module_id, hotplug->data, + payload_length - 0x02); break; case SVC_HOTUNPLUG_EVENT: /* Remove a module from the system */ + if (payload_length != 0x02) { + /* Hotunplug message is only 2 bytes big */ + dev_err(hd->parent, + "Illegal size of svc hotunplug message %d\n", + payload_length); + return; + } dev_dbg(hd->parent, "module id %d removed\n", module_id); gb_remove_module(hd, module_id); break; default: dev_err(hd->parent, - "received invalid hotplug message type %d\n", + "Received invalid hotplug message type %d\n", hotplug->hotplug_event); break; } } static void svc_ddb(struct svc_function_ddb *ddb, - struct greybus_host_device *hd) + int payload_length, struct greybus_host_device *hd) { + /* + * Need to properly validate payload_length once we start + * to handle ddb messages, but for now, we don't, so no need to check + * anything. + */ + /* What? An AP should not get this message */ dev_err(hd->parent, "Got an svc DDB message???\n"); } static void svc_power(struct svc_function_power *power, - struct greybus_host_device *hd) + int payload_length, struct greybus_host_device *hd) { u8 module_id = power->module_id; + /* + * The AP is only allowed to get a Battery Status message, not a Battery + * Status Request + */ if (power->power_type != SVC_POWER_BATTERY_STATUS) { - dev_err(hd->parent, "received invalid power type %d\n", + dev_err(hd->parent, "Received invalid power type %d\n", power->power_type); return; } + /* + * As struct struct svc_function_power_battery_status_request is 0 bytes + * big, we can just check the union of the whole structure to validate + * the size of this message. + */ + if (payload_length != sizeof(struct svc_function_power)) { + dev_err(hd->parent, + "Illegal size of svc power message %d\n", + payload_length); + return; + } + dev_dbg(hd->parent, "power status for module id %d is %d\n", module_id, power->status.status); @@ -163,14 +200,14 @@ static void svc_power(struct svc_function_power *power, } static void svc_epm(struct svc_function_epm *epm, - struct greybus_host_device *hd) + int payload_length, struct greybus_host_device *hd) { /* What? An AP should not get this message */ dev_err(hd->parent, "Got an EPM message???\n"); } static void svc_suspend(struct svc_function_suspend *suspend, - struct greybus_host_device *hd) + int payload_length, struct greybus_host_device *hd) { /* What? An AP should not get this message */ dev_err(hd->parent, "Got an suspend message???\n"); @@ -227,19 +264,19 @@ static void ap_process_event(struct work_struct *work) svc_management(&svc_msg->management, payload_length, hd); break; case SVC_FUNCTION_HOTPLUG: - svc_hotplug(&svc_msg->hotplug, hd); + svc_hotplug(&svc_msg->hotplug, payload_length, hd); break; case SVC_FUNCTION_DDB: - svc_ddb(&svc_msg->ddb, hd); + svc_ddb(&svc_msg->ddb, payload_length, hd); break; case SVC_FUNCTION_POWER: - svc_power(&svc_msg->power, hd); + svc_power(&svc_msg->power, payload_length, hd); break; case SVC_FUNCTION_EPM: - svc_epm(&svc_msg->epm, hd); + svc_epm(&svc_msg->epm, payload_length, hd); break; case SVC_FUNCTION_SUSPEND: - svc_suspend(&svc_msg->suspend, hd); + svc_suspend(&svc_msg->suspend, payload_length, hd); break; default: dev_err(hd->parent, "received invalid SVC function ID %d\n", diff --git a/drivers/staging/greybus/core.c b/drivers/staging/greybus/core.c index 9a232a0a9227..b4e3093e3899 100644 --- a/drivers/staging/greybus/core.c +++ b/drivers/staging/greybus/core.c @@ -346,7 +346,8 @@ static int create_cport(struct greybus_device *gdev, * Pass in a buffer that _should_ contain a Greybus module manifest * and spit out a greybus device structure. */ -void gb_add_module(struct greybus_host_device *hd, u8 module_id, u8 *data) +void gb_add_module(struct greybus_host_device *hd, u8 module_id, + u8 *data, int size) { // FIXME - should be the new module call... } diff --git a/drivers/staging/greybus/greybus.h b/drivers/staging/greybus/greybus.h index f804b198254d..855cb0e02bb7 100644 --- a/drivers/staging/greybus/greybus.h +++ b/drivers/staging/greybus/greybus.h @@ -288,7 +288,8 @@ const u8 *greybus_string(struct greybus_device *gdev, int id); /* Internal functions to gb module, move to internal .h file eventually. */ -void gb_add_module(struct greybus_host_device *hd, u8 module_id, u8 *data); +void gb_add_module(struct greybus_host_device *hd, u8 module_id, + u8 *data, int size); void gb_remove_module(struct greybus_host_device *hd, u8 module_id); int gb_new_ap_msg(u8 *data, int length, struct greybus_host_device *hd); -- 2.39.5