Skip to contentSkip to conversation

crypto: omap - switch from scatter_walk to plain offset

bug report Dan Carpenter 2 participants 2 messages Jun 2 Crypto API

lore
Share & tools
lore
#0 lore raw

Hello Eric Biggers,

Commit 42c5675c2f5b ("crypto: omap - switch from scatter_walk to plain offset") from Jan 5, 2025 (linux-next), leads to the following Smatch static checker warning:

drivers/crypto/omap-des.c:842 omap_des_irq() error: we previously assumed 'dd->in_sg' could be null (see line 844) drivers/crypto/omap-des.c:872 omap_des_irq() error: we previously assumed 'dd->out_sg' could be null (see line 874)

drivers/crypto/omap-des.c

    823 static irqreturn_t omap_des_irq(int irq, void *dev_id)
    824 {
    825         struct omap_des_dev *dd = dev_id;
    826         u32 status, i;
    827         u32 *src, *dst;
    828 
    829         status = omap_des_read(dd, DES_REG_IRQ_STATUS(dd));
    830         if (status & DES_REG_IRQ_DATA_IN) {
    831                 omap_des_write(dd, DES_REG_IRQ_ENABLE(dd), 0x0);
    832 
    833                 BUG_ON(!dd->in_sg);
    834 
    835                 BUG_ON(dd->in_sg_offset > dd->in_sg->length);
    836 
    837                 src = sg_virt(dd->in_sg) + dd->in_sg_offset;
    838 
    839                 for (i = 0; i < DES_BLOCK_WORDS; i++) {
    840                         omap_des_write(dd, DES_REG_DATA_N(dd, i), *src);
    841                         dd->in_sg_offset += 4;
--> 842                         if (dd->in_sg_offset == dd->in_sg->length) {
                                                        ^^^^^^^^^

Dereference.

    843                                 dd->in_sg = sg_next(dd->in_sg);

Imagine that sg_next() returns NULL, then the next iteration through the loop will crash.

    844                                 if (dd->in_sg) {
    845                                         dd->in_sg_offset = 0;
    846                                         src = sg_virt(dd->in_sg);
    847                                 }
    848                         } else {
    849                                 src++;
    850                         }
    851                 }
    852 
    853                 /* Clear IRQ status */

This email is a free service from the Smatch-CI project [smatch.sf.net].

regards,
dan carpenter

#1 lore raw
Quoted 38 lines · Dan Carpenter

Hello Eric Biggers,

Commit 42c5675c2f5b ("crypto: omap - switch from scatter_walk to plain offset") from Jan 5, 2025 (linux-next), leads to the following Smatch static checker warning:

drivers/crypto/omap-des.c:842 omap_des_irq() error: we previously assumed 'dd->in_sg' could be null (see line 844) drivers/crypto/omap-des.c:872 omap_des_irq() error: we previously assumed 'dd->out_sg' could be null (see line 874)

drivers/crypto/omap-des.c

    823 static irqreturn_t omap_des_irq(int irq, void *dev_id)
    824 {
    825         struct omap_des_dev *dd = dev_id;
    826         u32 status, i;
    827         u32 *src, *dst;
    828 
    829         status = omap_des_read(dd, DES_REG_IRQ_STATUS(dd));
    830         if (status & DES_REG_IRQ_DATA_IN) {
    831                 omap_des_write(dd, DES_REG_IRQ_ENABLE(dd), 0x0);
    832 
    833                 BUG_ON(!dd->in_sg);
    834 
    835                 BUG_ON(dd->in_sg_offset > dd->in_sg->length);
    836 
    837                 src = sg_virt(dd->in_sg) + dd->in_sg_offset;
    838 
    839                 for (i = 0; i < DES_BLOCK_WORDS; i++) {
    840                         omap_des_write(dd, DES_REG_DATA_N(dd, i), *src);
    841                         dd->in_sg_offset += 4;
--> 842                         if (dd->in_sg_offset == dd->in_sg->length) {
                                                        ^^^^^^^^^

Dereference.

    843                                 dd->in_sg = sg_next(dd->in_sg);

Imagine that sg_next() returns NULL, then the next iteration through the loop will crash.

Quoted 10 lines
    844                                 if (dd->in_sg) {
    845                                         dd->in_sg_offset = 0;
    846                                         src = sg_virt(dd->in_sg);
    847                                 }
    848                         } else {
    849                                 src++;
    850                         }
    851                 }
    852 
    853                 /* Clear IRQ status */

Pre-existing bug.

- Eric