[bug report] Input: ads7846 - add dummy command register clearing cycle

[bug report] Input: ads7846 - add dummy command register clearing cycle

[Dan Carpenter]

Hello Marek Vasut,

Commit 781a07da9bb9 ("Input: ads7846 - add dummy command register
clearing cycle") from Mar 20, 2024 (linux-next), leads to the
following Smatch static checker warning:

drivers/input/touchscreen/ads7846.c:412 ads7846_read12_ser() error: buffer overflow 'req->xfer' 6 <= 6
drivers/input/touchscreen/ads7846.c:413 ads7846_read12_ser() error: buffer overflow 'req->xfer' 6 <= 6
drivers/input/touchscreen/ads7846.c:416 ads7846_read12_ser() error: buffer overflow 'req->xfer' 6 <= 7
drivers/input/touchscreen/ads7846.c:417 ads7846_read12_ser() error: buffer overflow 'req->xfer' 6 <= 7
drivers/input/touchscreen/ads7846.c:418 ads7846_read12_ser() error: buffer overflow 'req->xfer' 6 <= 7
drivers/input/touchscreen/ads7846.c:418 ads7846_read12_ser() error: buffer overflow 'req->xfer' 6 <= 7
drivers/input/touchscreen/ads7846.c:419 ads7846_read12_ser() error: buffer overflow 'req->xfer' 6 <= 7

drivers/input/touchscreen/ads7846.c
    353 static int ads7846_read12_ser(struct device *dev, unsigned command)
    354 {
    355         struct spi_device *spi = to_spi_device(dev);
    356         struct ads7846 *ts = dev_get_drvdata(dev);
    357         struct ser_req *req;
    358         int status;
    359 
    360         req = kzalloc(sizeof *req, GFP_KERNEL);
    361         if (!req)
    362                 return -ENOMEM;
    363 
    364         spi_message_init(&req->msg);
    365 
    366         /* maybe turn on internal vREF, and let it settle */
    367         if (ts->use_internal) {
    368                 req->ref_on = REF_ON;
    369                 req->xfer[0].tx_buf = &req->ref_on;
    370                 req->xfer[0].len = 1;
    371                 spi_message_add_tail(&req->xfer[0], &req->msg);
    372 
    373                 req->xfer[1].rx_buf = &req->scratch;
    374                 req->xfer[1].len = 2;
    375 
    376                 /* for 1uF, settle for 800 usec; no cap, 100 usec.  */
    377                 req->xfer[1].delay.value = ts->vref_delay_usecs;
    378                 req->xfer[1].delay.unit = SPI_DELAY_UNIT_USECS;
    379                 spi_message_add_tail(&req->xfer[1], &req->msg);
    380 
    381                 /* Enable reference voltage */
    382                 command |= ADS_PD10_REF_ON;
    383         }
    384 
    385         /* Enable ADC in every case */
    386         command |= ADS_PD10_ADC_ON;
    387 
    388         /* take sample */
    389         req->command = (u8) command;
    390         req->xfer[2].tx_buf = &req->command;
    391         req->xfer[2].len = 1;
    392         spi_message_add_tail(&req->xfer[2], &req->msg);
    393 
    394         req->xfer[3].rx_buf = &req->sample;
    395         req->xfer[3].len = 2;
    396         spi_message_add_tail(&req->xfer[3], &req->msg);
    397 
    398         /* REVISIT:  take a few more samples, and compare ... */
    399 
    400         /* converter in low power mode & enable PENIRQ */
    401         req->ref_off = PWRDOWN;
    402         req->xfer[4].tx_buf = &req->ref_off;
    403         req->xfer[4].len = 1;
    404         spi_message_add_tail(&req->xfer[4], &req->msg);
    405 
    406         req->xfer[5].rx_buf = &req->scratch;
    407         req->xfer[5].len = 2;
    408         spi_message_add_tail(&req->xfer[5], &req->msg);
    409 
    410         /* clear the command register */
    411         req->scratch = 0;
--> 412         req->xfer[6].tx_buf = &req->scratch;
                     ^^^^^^^
The ->xfer[] array only has 6 elements.  Should we bump this to 8 elements?

    413         req->xfer[6].len = 1;
    414         spi_message_add_tail(&req->xfer[6], &req->msg);
    415 
    416         req->xfer[7].rx_buf = &req->scratch;
    417         req->xfer[7].len = 2;
    418         CS_CHANGE(req->xfer[7]);
    419         spi_message_add_tail(&req->xfer[7], &req->msg);
    420 
    421         mutex_lock(&ts->lock);
    422         ads7846_stop(ts);
    423         status = spi_sync(spi, &req->msg);
    424         ads7846_restart(ts);
    425         mutex_unlock(&ts->lock);
    426 
    427         if (status == 0) {
    428                 /* on-wire is a must-ignore bit, a BE12 value, then padding */
    429                 status = be16_to_cpu(req->sample);
    430                 status = status >> 3;
    431                 status &= 0x0fff;
    432         }
    433 
    434         kfree(req);
    435         return status;
    436 }

regards,
dan carpenter

Re: [bug report] Input: ads7846 - add dummy command register clearing cycle

[Marek Vasut]

On 11/12/24 10:08 AM, Dan Carpenter wrote:
> Hello Marek Vasut,
> 
> Commit 781a07da9bb9 ("Input: ads7846 - add dummy command register
> clearing cycle") from Mar 20, 2024 (linux-next), leads to the
> following Smatch static checker warning:
> 
> drivers/input/touchscreen/ads7846.c:412 ads7846_read12_ser() error: buffer overflow 'req->xfer' 6 <= 6
> drivers/input/touchscreen/ads7846.c:413 ads7846_read12_ser() error: buffer overflow 'req->xfer' 6 <= 6
> drivers/input/touchscreen/ads7846.c:416 ads7846_read12_ser() error: buffer overflow 'req->xfer' 6 <= 7
> drivers/input/touchscreen/ads7846.c:417 ads7846_read12_ser() error: buffer overflow 'req->xfer' 6 <= 7
> drivers/input/touchscreen/ads7846.c:418 ads7846_read12_ser() error: buffer overflow 'req->xfer' 6 <= 7
> drivers/input/touchscreen/ads7846.c:418 ads7846_read12_ser() error: buffer overflow 'req->xfer' 6 <= 7
> drivers/input/touchscreen/ads7846.c:419 ads7846_read12_ser() error: buffer overflow 'req->xfer' 6 <= 7
A fix for that was already posted by Nathan:

[PATCH] Input: ads7846 - Increase xfer array size in 'struct ser_req'

Sorry for the inconvenience.