Commit a8411784 authored by Javier Martinez Canillas's avatar Javier Martinez Canillas Committed by Lee Jones
Browse files

mfd: cros_ec: Use a zero-length array for command data 



Commit 1b84f2a4 ("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: default avatarGwendal Grignou <gwendal@chromium.org>
Signed-off-by: default avatarJavier Martinez Canillas <javier.martinez@collabora.co.uk>
Tested-by: default avatarHeiko Stuebner <heiko@sntech.de>
Acked-by: default avatarLee Jones <lee.jones@linaro.org>
Acked-by: default avatarOlof Johansson <olof@lixom.net>
Signed-off-by: default avatarLee Jones <lee.jones@linaro.org>
parent bb03ffb9

drivers/i2c/busses/i2c-cros-ec-tunnel.c

+30 −15
Original line number Diff line number Diff line
@@ -182,8 +182,9 @@ static int ec_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg i2c_msgs[], 
	const u16 bus_num = bus->remote_bus;

	int request_len;

	int response_len;

	int alloc_size;

	int result;

	struct cros_ec_command msg = { };

	struct cros_ec_command *msg;



	request_len = ec_i2c_count_message(i2c_msgs, num);

	if (request_len < 0) {
@@ -198,25 +199,39 @@ static int ec_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg i2c_msgs[], 
		return response_len;

	}



	result = ec_i2c_construct_message(msg.outdata, i2c_msgs, num, bus_num);

	if (result)

		return result;

	alloc_size = max(request_len, response_len);

	msg = kmalloc(sizeof(*msg) + alloc_size, GFP_KERNEL);

	if (!msg)

		return -ENOMEM;



	msg.version = 0;

	msg.command = EC_CMD_I2C_PASSTHRU;

	msg.outsize = request_len;

	msg.insize = response_len;

	result = ec_i2c_construct_message(msg->data, i2c_msgs, num, bus_num);

	if (result) {

		dev_err(dev, "Error constructing EC i2c message %d\n", result);

		goto exit;

	}



	result = cros_ec_cmd_xfer(bus->ec, &msg);

	if (result < 0)

		return result;

	msg->version = 0;

	msg->command = EC_CMD_I2C_PASSTHRU;

	msg->outsize = request_len;

	msg->insize = response_len;



	result = ec_i2c_parse_response(msg.indata, i2c_msgs, &num);

	if (result < 0)

		return result;

	result = cros_ec_cmd_xfer(bus->ec, msg);

	if (result < 0) {

		dev_err(dev, "Error transferring EC i2c message %d\n", result);

		goto exit;

	}



	result = ec_i2c_parse_response(msg->data, i2c_msgs, &num);

	if (result < 0) {

		dev_err(dev, "Error parsing EC i2c message %d\n", result);

		goto exit;

	}



	/* Indicate success by saying how many messages were sent */

	return num;

	result = num;

exit:

	kfree(msg);

	return result;

}



static u32 ec_i2c_functionality(struct i2c_adapter *adap)

drivers/input/keyboard/cros_ec_keyb.c

+19 −10
Original line number Diff line number Diff line
@@ -148,19 +148,28 @@ static void cros_ec_keyb_process(struct cros_ec_keyb *ckdev, 


static int cros_ec_keyb_get_state(struct cros_ec_keyb *ckdev, uint8_t *kb_state)

{

	int ret;

	struct cros_ec_command msg = {

		.command = EC_CMD_MKBP_STATE,

		.insize = ckdev->cols,

	};

	int ret = 0;

	struct cros_ec_command *msg;



	ret = cros_ec_cmd_xfer(ckdev->ec, &msg);

	if (ret < 0)

		return ret;

	msg = kmalloc(sizeof(*msg) + ckdev->cols, GFP_KERNEL);

	if (!msg)

		return -ENOMEM;



	memcpy(kb_state, msg.indata, ckdev->cols);

	msg->version = 0;

	msg->command = EC_CMD_MKBP_STATE;

	msg->insize = ckdev->cols;

	msg->outsize = 0;



	return 0;

	ret = cros_ec_cmd_xfer(ckdev->ec, msg);

	if (ret < 0) {

		dev_err(ckdev->dev, "Error transferring EC message %d\n", ret);

		goto exit;

	}



	memcpy(kb_state, msg->data, ckdev->cols);

exit:

	kfree(msg);

	return ret;

}



static irqreturn_t cros_ec_keyb_irq(int irq, void *data)

drivers/mfd/cros_ec.c

+20 −8
Original line number Diff line number Diff line
@@ -41,7 +41,7 @@ int cros_ec_prepare_tx(struct cros_ec_device *ec_dev, 
	out[2] = msg->outsize;

	csum = out[0] + out[1] + out[2];

	for (i = 0; i < msg->outsize; i++)

		csum += out[EC_MSG_TX_HEADER_BYTES + i] = msg->outdata[i];

		csum += out[EC_MSG_TX_HEADER_BYTES + i] = msg->data[i];

	out[EC_MSG_TX_HEADER_BYTES + msg->outsize] = (uint8_t)(csum & 0xff);



	return EC_MSG_TX_PROTO_BYTES + msg->outsize;
@@ -75,11 +75,20 @@ int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev, 
	ret = ec_dev->cmd_xfer(ec_dev, msg);

	if (msg->result == EC_RES_IN_PROGRESS) {

		int i;

		struct cros_ec_command status_msg = { };

		struct cros_ec_command *status_msg;

		struct ec_response_get_comms_status *status;



		status_msg.command = EC_CMD_GET_COMMS_STATUS;

		status_msg.insize = sizeof(*status);

		status_msg = kmalloc(sizeof(*status_msg) + sizeof(*status),

				     GFP_KERNEL);

		if (!status_msg) {

			ret = -ENOMEM;

			goto exit;

		}



		status_msg->version = 0;

		status_msg->command = EC_CMD_GET_COMMS_STATUS;

		status_msg->insize = sizeof(*status);

		status_msg->outsize = 0;



		/*

		 * Query the EC's status until it's no longer busy or
@@ -88,20 +97,23 @@ int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev, 
		for (i = 0; i < EC_COMMAND_RETRIES; i++) {

			usleep_range(10000, 11000);



			ret = ec_dev->cmd_xfer(ec_dev, &status_msg);

			ret = ec_dev->cmd_xfer(ec_dev, status_msg);

			if (ret < 0)

				break;



			msg->result = status_msg.result;

			if (status_msg.result != EC_RES_SUCCESS)

			msg->result = status_msg->result;

			if (status_msg->result != EC_RES_SUCCESS)

				break;



			status = (struct ec_response_get_comms_status *)

				 status_msg.indata;

				 status_msg->data;

			if (!(status->flags & EC_COMMS_STATUS_PROCESSING))

				break;

		}



		kfree(status_msg);

	}

exit:

	mutex_unlock(&ec_dev->lock);



	return ret;

drivers/mfd/cros_ec_i2c.c

+2 −2
Original line number Diff line number Diff line
@@ -76,7 +76,7 @@ static int cros_ec_cmd_xfer_i2c(struct cros_ec_device *ec_dev, 
	/* copy message payload and compute checksum */

	sum = out_buf[0] + out_buf[1] + out_buf[2];

	for (i = 0; i < msg->outsize; i++) {

		out_buf[3 + i] = msg->outdata[i];

		out_buf[3 + i] = msg->data[i];

		sum += out_buf[3 + i];

	}

	out_buf[3 + msg->outsize] = sum;
@@ -109,7 +109,7 @@ static int cros_ec_cmd_xfer_i2c(struct cros_ec_device *ec_dev, 
	/* copy response packet payload and compute checksum */

	sum = in_buf[0] + in_buf[1];

	for (i = 0; i < len; i++) {

		msg->indata[i] = in_buf[2 + i];

		msg->data[i] = in_buf[2 + i];

		sum += in_buf[2 + i];

	}

	dev_dbg(ec_dev->dev, "packet: %*ph, sum = %02x\n",

drivers/mfd/cros_ec_spi.c

+1 −1
Original line number Diff line number Diff line
@@ -299,7 +299,7 @@ static int cros_ec_cmd_xfer_spi(struct cros_ec_device *ec_dev, 
	for (i = 0; i < len; i++) {

		sum += ptr[i + 2];

		if (ec_msg->insize)

			ec_msg->indata[i] = ptr[i + 2];

			ec_msg->data[i] = ptr[i + 2];

	}

	sum &= 0xff;

CRA Git | Maintained and supported by SUSTech CRA and CCSE