[bug report] mfd: tps6594: Add driver for TI TPS6594 PMIC

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello Julien Panis,

The patch 325bec7157b3: "mfd: tps6594: Add driver for TI TPS6594
PMIC" from May 11, 2023, leads to the following Smatch static checker
warning:

	drivers/mfd/tps6594-i2c.c:159 tps6594_i2c_write()
	warn: 'count - 2' negative user limit promoted to high

drivers/mfd/tps6594-i2c.c
    142 static int tps6594_i2c_write(void *context, const void *data, size_t count)
    143 {
    144         struct i2c_client *client = context;
    145         struct tps6594 *tps = i2c_get_clientdata(client);
    146         struct i2c_msg msg;
    147         const u8 *bytes = data;
    148         u8 *buf;
    149         const u8 page = bytes[1];
    150         const u8 reg = bytes[0];
    151         int ret = 0;
    152         int i;
    153 
    154         if (tps->use_crc) {
    155                 /*
    156                  * Auto-increment feature does not support CRC protocol.
    157                  * Converts the bulk write operation into a series of single write operations.
    158                  */
--> 159                 for (i = 0 ; ret == 0 && i < count - 2 ; i++)

TL;DR Smatch is correctly complaining that "count - 2" can underflow.

Smatch finds this code hard to parse.  It says that "count" is a user
controlled number between 0-u64max which has an upperbound (but the
upper bound is variable instead of a constant).

Can count actually be zero?  Smatch is normally good at tracking that...
The other limitation that Smatch does not track is from regmap_raw_write()

	if (val_len % map->format.val_bytes)

In this case the value of map->format.val_bytes comes from:

drivers/mfd/tps6594-i2c.c
   186  static const struct regmap_config tps6594_i2c_regmap_config = {
   187          .reg_bits = 16,
   188          .val_bits = 8,
                ^^^^^^^^^^^^^^

   189          .max_register = TPS6594_REG_DWD_FAIL_CNT_REG,
   190          .volatile_reg = tps6594_is_volatile_reg,
   191          .read = tps6594_i2c_read,
   192          .write = tps6594_i2c_write,
   193  };

So we either need to add some special handling for count == 1 or change
.val_bits = 16 or something.

    160                         ret = tps6594_i2c_reg_write_with_crc(client, page, reg + i, bytes[i + 2]);
    161 
    162                 return ret;
    163         }
    164 
    165         /* Setup buffer: page byte is not sent */
    166         buf = kzalloc(--count, GFP_KERNEL);
    167         if (!buf)
    168                 return -ENOMEM;
    169 
    170         buf[0] = reg;
    171         for (i = 0 ; i < count - 1 ; i++)
    172                 buf[i + 1] = bytes[i + 2];
    173 
    174         /* Write register and data: I2C address = I2C base address + Page index */
    175         msg.addr = client->addr + page;
    176         msg.flags = client->flags & I2C_M_TEN;
    177         msg.len = count;
    178         msg.buf = buf;
    179 
    180         ret = tps6594_i2c_transfer(client->adapter, &msg, 1);
    181 
    182         kfree(buf);
    183         return ret;
    184 }

regards,
dan carpenter



[Index of Archives]     [Kernel Development]     [Kernel Announce]     [Kernel Newbies]     [Linux Networking Development]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Device Mapper]

  Powered by Linux