Skip to content

Commit

Permalink
platform/chrome: cros_ec_dev - Fix security issue
Browse files Browse the repository at this point in the history
Prevent memory scribble by checking that ioctl buffer size parameters
are sane.
Without this check, on 32 bits system, if .insize = 0xffffffff - 20 and
.outsize the amount to scribble, we would overflow, allocate a small
amounts and be able to write outside of the malloc'ed area.
Adding a hard limit allows argument checking of the ioctl. With the
current EC, it is expected .insize and .outsize to be at around 512 bytes
or less.

Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
Signed-off-by: Olof Johansson <olof@lixom.net>
  • Loading branch information
gwendalcr authored and olofj committed May 11, 2016
1 parent 492ef78 commit 5d749d0
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 4 deletions.
4 changes: 4 additions & 0 deletions drivers/platform/chrome/cros_ec_dev.c
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,10 @@ static long ec_device_ioctl_xcmd(struct cros_ec_dev *ec, void __user *arg)
if (copy_from_user(&u_cmd, arg, sizeof(u_cmd)))
return -EFAULT;

if ((u_cmd.outsize > EC_MAX_MSG_BYTES) ||
(u_cmd.insize > EC_MAX_MSG_BYTES))
return -EINVAL;

s_cmd = kmalloc(sizeof(*s_cmd) + max(u_cmd.outsize, u_cmd.insize),
GFP_KERNEL);
if (!s_cmd)
Expand Down
4 changes: 2 additions & 2 deletions drivers/platform/chrome/cros_ec_proto.c
Original file line number Diff line number Diff line change
Expand Up @@ -298,8 +298,8 @@ int cros_ec_query_all(struct cros_ec_device *ec_dev)
ec_dev->max_response = EC_PROTO2_MAX_PARAM_SIZE;
ec_dev->max_passthru = 0;
ec_dev->pkt_xfer = NULL;
ec_dev->din_size = EC_MSG_BYTES;
ec_dev->dout_size = EC_MSG_BYTES;
ec_dev->din_size = EC_PROTO2_MSG_BYTES;
ec_dev->dout_size = EC_PROTO2_MSG_BYTES;
} else {
/*
* It's possible for a test to occur too early when
Expand Down
6 changes: 4 additions & 2 deletions include/linux/mfd/cros_ec.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,11 @@ enum {
EC_MSG_TX_TRAILER_BYTES,
EC_MSG_RX_PROTO_BYTES = 3,

/* Max length of messages */
EC_MSG_BYTES = EC_PROTO2_MAX_PARAM_SIZE +
/* Max length of messages for proto 2*/
EC_PROTO2_MSG_BYTES = EC_PROTO2_MAX_PARAM_SIZE +
EC_MSG_TX_PROTO_BYTES,

EC_MAX_MSG_BYTES = 64 * 1024,
};

/*
Expand Down

0 comments on commit 5d749d0

Please sign in to comment.