From a841178445bb72a3d566b4e6ab9d19e9b002eb47 Mon Sep 17 00:00:00 2001 From: Javier Martinez Canillas Date: Tue, 9 Jun 2015 13:04:42 +0200 Subject: mfd: cros_ec: Use a zero-length array for command data Commit 1b84f2a4cd4a ("mfd: cros_ec: Use fixed size arrays to transfer data with the EC") modified the struct cros_ec_command fields to not use pointers for the input and output buffers and use fixed length arrays instead. This change was made because the cros_ec ioctl API uses that struct cros_ec_command to allow user-space to send commands to the EC and to get data from the EC. So using pointers made the API not 64-bit safe. Unfortunately this approach was not flexible enough for all the use-cases since there may be a need to send larger commands on newer versions of the EC command protocol. So to avoid to choose a constant length that it may be too big for most commands and thus wasting memory and CPU cycles on copy from and to user-space or having a size that is too small for some big commands, use a zero-length array that is both 64-bit safe and flexible. The same buffer is used for both output and input data so the maximum of these values should be used to allocate it. Suggested-by: Gwendal Grignou Signed-off-by: Javier Martinez Canillas Tested-by: Heiko Stuebner Acked-by: Lee Jones Acked-by: Olof Johansson Signed-off-by: Lee Jones --- drivers/platform/chrome/cros_ec_lightbar.c | 152 +++++++++++++++++++---------- 1 file changed, 103 insertions(+), 49 deletions(-) (limited to 'drivers/platform/chrome/cros_ec_lightbar.c') diff --git a/drivers/platform/chrome/cros_ec_lightbar.c b/drivers/platform/chrome/cros_ec_lightbar.c index b4ff47a9069a..560e5d41b7ae 100644 --- a/drivers/platform/chrome/cros_ec_lightbar.c +++ b/drivers/platform/chrome/cros_ec_lightbar.c @@ -31,6 +31,7 @@ #include #include #include +#include #include "cros_ec_dev.h" @@ -91,54 +92,79 @@ out: return ret; } -#define INIT_MSG(P, R) { \ - .command = EC_CMD_LIGHTBAR_CMD, \ - .outsize = sizeof(*P), \ - .insize = sizeof(*R), \ - } +static struct cros_ec_command *alloc_lightbar_cmd_msg(void) +{ + struct cros_ec_command *msg; + int len; + + len = max(sizeof(struct ec_params_lightbar), + sizeof(struct ec_response_lightbar)); + + msg = kmalloc(sizeof(*msg) + len, GFP_KERNEL); + if (!msg) + return NULL; + + msg->version = 0; + msg->command = EC_CMD_LIGHTBAR_CMD; + msg->outsize = sizeof(struct ec_params_lightbar); + msg->insize = sizeof(struct ec_response_lightbar); + + return msg; +} static int get_lightbar_version(struct cros_ec_device *ec, uint32_t *ver_ptr, uint32_t *flg_ptr) { struct ec_params_lightbar *param; struct ec_response_lightbar *resp; - struct cros_ec_command msg = INIT_MSG(param, resp); + struct cros_ec_command *msg; int ret; - param = (struct ec_params_lightbar *)msg.outdata; - param->cmd = LIGHTBAR_CMD_VERSION; - ret = cros_ec_cmd_xfer(ec, &msg); - if (ret < 0) + msg = alloc_lightbar_cmd_msg(); + if (!msg) return 0; - switch (msg.result) { + param = (struct ec_params_lightbar *)msg->data; + param->cmd = LIGHTBAR_CMD_VERSION; + ret = cros_ec_cmd_xfer(ec, msg); + if (ret < 0) { + ret = 0; + goto exit; + } + + switch (msg->result) { case EC_RES_INVALID_PARAM: /* Pixel had no version command. */ if (ver_ptr) *ver_ptr = 0; if (flg_ptr) *flg_ptr = 0; - return 1; + ret = 1; + goto exit; case EC_RES_SUCCESS: - resp = (struct ec_response_lightbar *)msg.indata; + resp = (struct ec_response_lightbar *)msg->data; /* Future devices w/lightbars should implement this command */ if (ver_ptr) *ver_ptr = resp->version.num; if (flg_ptr) *flg_ptr = resp->version.flags; - return 1; + ret = 1; + goto exit; } /* Anything else (ie, EC_RES_INVALID_COMMAND) - no lightbar */ - return 0; + ret = 0; +exit: + kfree(msg); + return ret; } static ssize_t version_show(struct device *dev, struct device_attribute *attr, char *buf) { - uint32_t version, flags; + uint32_t version = 0, flags = 0; struct cros_ec_device *ec = dev_get_drvdata(dev); int ret; @@ -158,8 +184,7 @@ static ssize_t brightness_store(struct device *dev, const char *buf, size_t count) { struct ec_params_lightbar *param; - struct ec_response_lightbar *resp; - struct cros_ec_command msg = INIT_MSG(param, resp); + struct cros_ec_command *msg; int ret; unsigned int val; struct cros_ec_device *ec = dev_get_drvdata(dev); @@ -167,21 +192,30 @@ static ssize_t brightness_store(struct device *dev, if (kstrtouint(buf, 0, &val)) return -EINVAL; - param = (struct ec_params_lightbar *)msg.outdata; + msg = alloc_lightbar_cmd_msg(); + if (!msg) + return -ENOMEM; + + param = (struct ec_params_lightbar *)msg->data; param->cmd = LIGHTBAR_CMD_BRIGHTNESS; param->brightness.num = val; ret = lb_throttle(); if (ret) - return ret; + goto exit; - ret = cros_ec_cmd_xfer(ec, &msg); + ret = cros_ec_cmd_xfer(ec, msg); if (ret < 0) - return ret; + goto exit; - if (msg.result != EC_RES_SUCCESS) - return -EINVAL; + if (msg->result != EC_RES_SUCCESS) { + ret = -EINVAL; + goto exit; + } - return count; + ret = count; +exit: + kfree(msg); + return ret; } @@ -196,12 +230,15 @@ static ssize_t led_rgb_store(struct device *dev, struct device_attribute *attr, const char *buf, size_t count) { struct ec_params_lightbar *param; - struct ec_response_lightbar *resp; - struct cros_ec_command msg = INIT_MSG(param, resp); + struct cros_ec_command *msg; struct cros_ec_device *ec = dev_get_drvdata(dev); unsigned int val[4]; int ret, i = 0, j = 0, ok = 0; + msg = alloc_lightbar_cmd_msg(); + if (!msg) + return -ENOMEM; + do { /* Skip any whitespace */ while (*buf && isspace(*buf)) @@ -215,7 +252,7 @@ static ssize_t led_rgb_store(struct device *dev, struct device_attribute *attr, return -EINVAL; if (i == 4) { - param = (struct ec_params_lightbar *)msg.outdata; + param = (struct ec_params_lightbar *)msg->data; param->cmd = LIGHTBAR_CMD_RGB; param->rgb.led = val[0]; param->rgb.red = val[1]; @@ -231,12 +268,14 @@ static ssize_t led_rgb_store(struct device *dev, struct device_attribute *attr, return ret; } - ret = cros_ec_cmd_xfer(ec, &msg); + ret = cros_ec_cmd_xfer(ec, msg); if (ret < 0) - return ret; + goto exit; - if (msg.result != EC_RES_SUCCESS) - return -EINVAL; + if (msg->result != EC_RES_SUCCESS) { + ret = -EINVAL; + goto exit; + } i = 0; ok = 1; @@ -248,6 +287,8 @@ static ssize_t led_rgb_store(struct device *dev, struct device_attribute *attr, } while (*buf); +exit: + kfree(msg); return (ok && i == 0) ? count : -EINVAL; } @@ -261,42 +302,55 @@ static ssize_t sequence_show(struct device *dev, { struct ec_params_lightbar *param; struct ec_response_lightbar *resp; - struct cros_ec_command msg = INIT_MSG(param, resp); + struct cros_ec_command *msg; int ret; struct cros_ec_device *ec = dev_get_drvdata(dev); - param = (struct ec_params_lightbar *)msg.outdata; + msg = alloc_lightbar_cmd_msg(); + if (!msg) + return -ENOMEM; + + param = (struct ec_params_lightbar *)msg->data; param->cmd = LIGHTBAR_CMD_GET_SEQ; ret = lb_throttle(); if (ret) - return ret; + goto exit; - ret = cros_ec_cmd_xfer(ec, &msg); + ret = cros_ec_cmd_xfer(ec, msg); if (ret < 0) - return ret; + goto exit; - if (msg.result != EC_RES_SUCCESS) - return scnprintf(buf, PAGE_SIZE, - "ERROR: EC returned %d\n", msg.result); + if (msg->result != EC_RES_SUCCESS) { + ret = scnprintf(buf, PAGE_SIZE, + "ERROR: EC returned %d\n", msg->result); + goto exit; + } - resp = (struct ec_response_lightbar *)msg.indata; + resp = (struct ec_response_lightbar *)msg->data; if (resp->get_seq.num >= ARRAY_SIZE(seqname)) - return scnprintf(buf, PAGE_SIZE, "%d\n", resp->get_seq.num); + ret = scnprintf(buf, PAGE_SIZE, "%d\n", resp->get_seq.num); else - return scnprintf(buf, PAGE_SIZE, "%s\n", - seqname[resp->get_seq.num]); + ret = scnprintf(buf, PAGE_SIZE, "%s\n", + seqname[resp->get_seq.num]); + +exit: + kfree(msg); + return ret; } static ssize_t sequence_store(struct device *dev, struct device_attribute *attr, const char *buf, size_t count) { struct ec_params_lightbar *param; - struct ec_response_lightbar *resp; - struct cros_ec_command msg = INIT_MSG(param, resp); + struct cros_ec_command *msg; unsigned int num; int ret, len; struct cros_ec_device *ec = dev_get_drvdata(dev); + msg = alloc_lightbar_cmd_msg(); + if (!msg) + return -ENOMEM; + for (len = 0; len < count; len++) if (!isalnum(buf[len])) break; @@ -311,18 +365,18 @@ static ssize_t sequence_store(struct device *dev, struct device_attribute *attr, return ret; } - param = (struct ec_params_lightbar *)msg.outdata; + param = (struct ec_params_lightbar *)msg->data; param->cmd = LIGHTBAR_CMD_SEQ; param->seq.num = num; ret = lb_throttle(); if (ret) return ret; - ret = cros_ec_cmd_xfer(ec, &msg); + ret = cros_ec_cmd_xfer(ec, msg); if (ret < 0) return ret; - if (msg.result != EC_RES_SUCCESS) + if (msg->result != EC_RES_SUCCESS) return -EINVAL; return count; -- cgit v1.2.3