Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Why is sprintf with possible buffer overflow allowed in Linux hwmon?

I've seen the following snippet of code used repeatedly for Linux hwmon devices:

return sprintf(buf, "%d\n", in_input);

Where buf is a pointer to char char *buf, and in_input usually an int or u16. The purpose is to copy the value read back from the device to the sysfs file created for this device attribute.

As an example, you can take a look at Linux/drivers/hwmon/mcp3021.c (or any hwmon device previous to 4.9 kernel really). You can see the function at line 81 returns a u16, and on line 99 the code stores the u16 into the char *buf.

 81 static inline u16 volts_from_reg(struct mcp3021_data *data, u16 val)
 82 {
 83         return DIV_ROUND_CLOSEST(data->vdd * val, 1 << data->output_res);
 84 }
 85 
 86 static ssize_t show_in_input(struct device *dev, struct device_attribute *attr,
 87                 char *buf)
 88 {
 89         struct i2c_client *client = to_i2c_client(dev);
 90         struct mcp3021_data *data = i2c_get_clientdata(client);
 91         int reg, in_input;
 92 
 93         reg = mcp3021_read16(client);
 94         if (reg < 0)
 95                 return reg;
 96 
 97         in_input = volts_from_reg(data, reg);
 98 
 99         return sprintf(buf, "%d\n", in_input);
100 }

Wouldn't this code always cause buffer overflow? We are always storing a u16 into a buffer allocated for char 8-bits. Why is this permitted in Linux device drivers?

Note that with my driver that does the exact same thing, sysfs displays the returned value correctly even though it cannot possibly be stored as a char (8-bits). So wondering if the char * representation is not accurate?

like image 362
Splaty Avatar asked Jan 14 '17 00:01

Splaty


1 Answers

According to the documentation in sysfs.txt, the buffer passed to the show function is of size PAGE_SIZE:

sysfs allocates a buffer of size (PAGE_SIZE) and passes it to the method. Sysfs will call the method exactly once for each read or write.

Since PAGE_SIZE is certainly bigger than the length of a small integer, there is no practical possibility of a buffer overflow.

like image 172
rici Avatar answered Oct 19 '22 06:10

rici