Android: Race condition in sec_ts touchscreen sysfs interface  

The Samsung touchscreen driver exposes a sysfs interface though which the driver may be configured.

Two such entries are present under:
    /sys/devices/virtual/sec/sec_ts/sec_ts_regreadsize
    /sys/devices/virtual/sec/sec_ts/sec_ts_regread

These entries may be written to and read from. The "sec_ts_regreadsize" entry is used in order to configure the size of the register read buffer. Once the size is configured, the data of the registers can be read by reading the "sec_ts_regread" entry.

A race condition exists in the handling of these two entries. Here is the code responsible for handling the writes to "sec_ts_regreadsize":

static ssize_t sec_ts_regreadsize_store(struct device *dev, struct device_attribute *attr, const char *buf, size_t size)
{
    lv1cmd = buf[0];
    lv1_readsize = ((unsigned int)buf[4] << 24) |
                   ((unsigned int)buf[3] << 16) |
                   ((unsigned int)buf[2] <<  8) |
                   ((unsigned int)buf[1] <<  0);
    lv1_readoffset = 0;
    lv1_readremain = 0;
    return size;
}

Note that this function does not acquire any lock in order to prevent concurrent execution. Also, note that "lv1_readsize" is a static global variable.

Next, here is the code responsible for reading the "sec_ts_regread" entry:

1. static ssize_t sec_ts_regread_show(struct device *dev, struct device_attribute *attr, char *buf)
2. {
3.     struct sec_ts_data *ts = dev_get_drvdata(dev);
4.     ...
5.     read_lv1_buff = (u8 *)kzalloc(sizeof(u8)*lv1_readsize, GFP_KERNEL);
6.     if (!read_lv1_buff) {
7.         tsp_debug_err(true, &ts->client->dev, "%s kzalloc failed\n", __func__);
8.         goto malloc_err;
9.     }
10.
11.    mutex_lock(&ts->device_mutex);
12.    remain = lv1_readsize;
13.    offset = 0;
14.    do
15.    {
16.        if(remain >= ts->i2c_burstmax)
17.            length = ts->i2c_burstmax;
18.        else
19.            length = remain;
20.
21.        if( offset == 0 )
22.            ret = sec_ts_i2c_read(ts, lv1cmd, &read_lv1_buff[offset], length);
23.        else
24.            ret = sec_ts_i2c_read_bulk(ts, &read_lv1_buff[offset], length);
25.
26.        ...
27.        remain -= length;
28.        offset += length;
29.    } while(remain > 0);
30.
31.    tsp_debug_info(true, &ts->client->dev, "%s: lv1_readsize = %d \n", __func__, lv1_readsize);
32.    memcpy(buf, read_lv1_buff + lv1_readoffset, lv1_readsize);
33.    ...
34. }

Since there is no synchronization mechanism preventing "sec_ts_regreadsize_store" from being called while "sec_ts_regread_show" is being executed, a race condition exists which allows the value of lv1_readsize to be modified during the execution of "sec_ts_regread_show".

Here is one such possible schedule which would result in kernel memory corruption:
    1. Thread A: Writes to "sec_ts_regreadsize" in order to set "lv1_readsize" to 10.
    2. Thread A: Attempts to read "sec_ts_regread"
    3. Thread A: Lines 1-10 are executed, resulting in an allocation of "read_lv1_buff" with size 10
    4. Thread B: Writes to "sec_ts_regreadsize" in order to set "lv1_readsize" to 20.
    5. Thread A: Lines 11-34 now proceed, with lv1_readsize=20
    6. Thread A: The read loop continues to read (lines 21-24) from the i2c bus into "read_lv1_buff". Since "read_lv1_buff" is only 10 bytes long, this results in an overflow.

I've statically verified this issue on an SM-G935F device. The open-source kernel package I analysed was "SM-G935F_MM_Opensource".

The sysfs entries mentioned above have UID/GID "root". The SELinux context for these entries is: "u:object_r:sysfs_sec:s0".

According to the default SELinux rules as present on the SM-G935F (version XXS1APG3), the following contexts may access these files:

   allow shell sysfs_sec : file { read open } ; 
   allow system_app sysfs_sec : file { ioctl read write getattr lock append open } ; 
   allow rild sysfs_sec : file { ioctl read write getattr lock append open } ; 
   allow system_app sysfs_sec : dir { ioctl read write getattr add_name remove_name search open } ; 
   allow diagexe sysfs_sec : file { ioctl read write getattr lock append open } ; 
   allow at_distributor sysfs_sec : file { ioctl read write getattr setattr lock append open } ; 

This bug is subject to a 90 day disclosure deadline. If 90 days elapse
without a broadly available patch, then the bug report will automatically
become visible to the public.




Found by: laginimaineb