Linux 4.6 Double-Fetch Race Condition / Buffer Overflow
Posted on 30 November -0001
<HTML><HEAD><TITLE>Linux 4.6 Double-Fetch Race Condition / Buffer Overflow</TITLE><META http-equiv="Content-Type" content="text/html; charset=utf-8"></HEAD><BODY>Hi, I found this double-fetch bug in Linux-4.6/drivers/platform/chrome/cros_ec_dev.c when I was inspecting the source code. In function ec_device_ioctl_xcmd(), the driver fetches user space data by pointer arg via copy_from_user(), and this happens twice at line 137 and line 145 respectively. The first fetched value (stored in u_cmd) is used to get the in_size and out_size elements and allocation a buffer (s_cmd) at line 140 so as to copy the whole message to driver later at line 145, which means the copy size of the whole message（s_cmd）is based on the old value (u_cmd.outside) that came from the first fetch. Besides, the whole message copied at the second fetch also contains the elements of in_size and out_size, which we call as the new values. The new values from the second fetch might be changed by another user thread under race condition, which will result in a double-fetch bug when the inconsistent values are used. We can see this happening by tracking how the whole message from the second fetch is used. First of all, s_cmd is passed to cros_ec_cmd_xfer（）for further process at line 151. While within cros_ec_cmd_xfer（）（in file cros_ec_proto.c）, msg is firstly used for some validity checking, then again passed to function send_command() at line 377. Although msg->outsize and msg->insize are checked here, they are only checked for vialidity by comparing with some max values, rather than checking the consistency to prevent double-fetch bug. Then we step into send_command() at line 58 of file cros_ec_proto.c, and msg is passed to ec_dev->pkt_xfer(ec_dev, msg) and ec_dev->cmd_xfer(ec_dev, msg) respectively. These two are actually pointers to function cros_ec_pkt_xfer_i2c（）and cros_ec_cmd_xfer_i2c（）respectively. In function cros_ec_cmd_xfer_i2c（）(line 187 of cros_ec_i2c.c), msg->outsize is used again without checking to control a loop (line233), if msg->outsize is changed to a greater value than the original one, memory access msg->data[i] at line 234 will beyond array boundary. While for cros_ec_pkt_xfer_i2c(), things are similar, except the loop moves into cros_ec_prepare_tx() at line 98. I noticed there were some checking during the tracking, however, the checking only compared the value with the max value, such as BUG_ON(msg->outsize > EC_PROTO2_MAX_PARAM_SIZE)，and this does not guarantee the consistency of the value. As long as the value is changed to a bigger value but not greater than the max value under race condition, problems still happen. When the message is switched to a longer one after the first fetch, only part of the message will be copied to the kernel by the second fetch based on the old value of size. However, when using the message, based on the size value of the new message, it would be used as a longer message, and memory access will beyond the array boundary. I am looking forward to a reply to confirm this, thank you! Kind regards Pengfei </BODY></HTML>
