mantis: check for errors on readl inside loop
Subsystems (5)
PATCH 1/6 mantis: check for errors on readl inside loop
As warned by smatch:
drivers/media/pci/mantis/mantis_uart.c:105 mantis_uart_work() warn: this loop depends on readl() succeeding
If readl() fails, this could lead into an endless loop. Avoid that. We might instead add some timeout logic, but it readl() is failing, then something really wrong is happening.
While here, remove two defines that are only used once.
--- drivers/media/pci/mantis/mantis_common.h | 7 ++----- drivers/media/pci/mantis/mantis_uart.c | 4 ++-- 2 files changed, 4 insertions(+), 7 deletions(-)
drivers/media/pci/mantis/mantis_common.h +2 -5
@@ -54,11 +54,8 @@ } \ } while(0) -#define mwrite (dat, addr) writel((dat), addr) -#define mread (addr) readl(addr) - -#define mmwrite (dat, addr) mwrite((dat), (mantis->mmio + (addr))) -#define mmread (addr) mread(mantis->mmio + (addr)) +#define mmwrite (dat, addr) writel((dat), (mantis->mmio + (addr))) +#define mmread (addr) readl(mantis->mmio + (addr))#define MANTIS_TS_188 0#define MANTIS_TS_204 1
drivers/media/pci/mantis/mantis_uart.c +2 -2
@@ -91,7 +91,7 @@ static void mantis_uart_read(struct mantis_pci *mantis)static void mantis_uart_work (struct work_struct *work) { struct mantis_pci *mantis = container_of(work, struct mantis_pci, uart_work); -u32 stat; +int stat; stat =mmread (MANTIS_UART_STAT );
@@ -102,7 +102,7 @@ static void mantis_uart_work(struct work_struct *work)* MANTIS_UART_RXFIFO_DATA is only set if at least * config->bytes + 1 bytes are in the FIFO. */ -while (stat &MANTIS_UART_RXFIFO_DATA ) { +while ((stat >=0 ) && (stat &MANTIS_UART_RXFIFO_DATA )) {mantis_uart_read (mantis); stat =mmread (MANTIS_UART_STAT ); }
Signature · 1 line
2.5.0
PATCH 2/6 ddcore: avoid endless loop if readl() fails
As warned by smatch:
drivers/media/pci/ddbridge/ddbridge-core.c:1351 flashio() warn: this loop depends on readl() succeeding drivers/media/pci/ddbridge/ddbridge-core.c:1371 flashio() warn: this loop depends on readl() succeeding
Let the loop be interrupted too if something really bad happens and readl() fails.
--- drivers/media/pci/ddbridge/ddbridge-core.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-)
drivers/media/pci/ddbridge/ddbridge-core.c +13 -8
@@ -1339,6 +1339,7 @@ static irqreturn_t irq_handler(int irq, void *dev_id)static int flashio (struct ddb *dev,u8 *wbuf,u32 wlen,u8 *rbuf,u32 rlen) {u32 data, shift; +int val;if (wlen >4 )ddbwritel (1 ,SPI_CONTROL );
@@ -1348,8 +1349,9 @@ static int flashio(struct ddb *dev, u8 *wbuf, u32 wlen, u8 *rbuf, u32 rlen) wbuf +=4 ; wlen -=4 ;ddbwritel (data,SPI_DATA ); -while (ddbreadl (SPI_CONTROL ) &0x0004 ) - ; +do { + val =ddbreadl (SPI_CONTROL ); + }while (val >=0 && val &0x0004 ); }if (rlen)
@@ -1368,8 +1370,9 @@ static int flashio(struct ddb *dev, u8 *wbuf, u32 wlen, u8 *rbuf, u32 rlen)if (shift) data <<= shift;ddbwritel (data,SPI_DATA ); -while (ddbreadl (SPI_CONTROL ) &0x0004 ) - ; +do { + val =ddbreadl (SPI_CONTROL ); + }while (val >=0 && val &0x0004 );if (!rlen) {ddbwritel (0 ,SPI_CONTROL );
@@ -1380,8 +1383,9 @@ static int flashio(struct ddb *dev, u8 *wbuf, u32 wlen, u8 *rbuf, u32 rlen)while (rlen >4 ) {ddbwritel (0xffffffff ,SPI_DATA ); -while (ddbreadl (SPI_CONTROL ) &0x0004 ) - ; +do { + val =ddbreadl (SPI_CONTROL ); + }while (val >=0 && val &0x0004 ); data =ddbreadl (SPI_DATA ); *(u32 *) rbuf =swab32 (data); rbuf +=4 ;
@@ -1389,8 +1393,9 @@ static int flashio(struct ddb *dev, u8 *wbuf, u32 wlen, u8 *rbuf, u32 rlen) }ddbwritel (0x0003 | ((rlen << (8 +3 )) &0x1F00 ),SPI_CONTROL );ddbwritel (0xffffffff ,SPI_DATA ); -while (ddbreadl (SPI_CONTROL ) &0x0004 ) - ; +do { + val =ddbreadl (SPI_CONTROL ); + }while (val >=0 && val &0x0004 ); data =ddbreadl (SPI_DATA );ddbwritel (0 ,SPI_CONTROL );
Signature · 1 line
2.5.0
PATCH 3/6 mceusb: use %*ph for small buffer dumps
In reply to Mauro Carvalho ChehabIt makes the printk cleaner. As a side efect, it also fixes those smatch
warnings:
drivers/media/rc/mceusb.c:590 mceusb_dev_printdata() warn: argument 6 to %02x specifier has type 'char' drivers/media/rc/mceusb.c:590 mceusb_dev_printdata() warn: argument 7 to %02x specifier has type 'char'
--- drivers/media/rc/mceusb.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
drivers/media/rc/mceusb.c +2 -3
@@ -587,9 +587,8 @@ static void mceusb_dev_printdata(struct mceusb_dev *ir, char *buf,if (len ==2 )dev_dbg (dev,"Get hw/sw rev?" );else -dev_dbg (dev,"hw/sw rev 0x%02x 0x%02x 0x%02x 0x%02x" , - data1, data2, - buf[start +4 ], buf[start +5 ]); +dev_dbg (dev,"hw/sw rev %*ph" , +4 , &buf[start +2 ]);break ;case MCE_CMD_RESUME :dev_dbg (dev,"Device resume requested" );
Signature · 1 line
2.5.0
PATCH 4/6 st-rc: prevent a endless loop
In reply to Mauro Carvalho ChehabAs warned by smatch:
drivers/media/rc/st_rc.c:110 st_rc_rx_interrupt() warn: this loop depends on readl() succeeding
as readl() might fail, with likely means some unrecovered error, let's loop only if it succeeds.
--- drivers/media/rc/st_rc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/media/rc/st_rc.c +2 -2
@@ -99,7 +99,7 @@ static irqreturn_t st_rc_rx_interrupt(int irq, void *data)unsigned int symbol, mark =0 ;struct st_rc_device *dev = data;int last_symbol =0 ; -u32 status; +int status;DEFINE_IR_RAW_EVENT (ev);if (dev->irq_wake)
@@ -107,7 +107,7 @@ static irqreturn_t st_rc_rx_interrupt(int irq, void *data) status =readl (dev->rx_base +IRB_RX_STATUS ); -while (status & (IRB_FIFO_NOT_EMPTY |IRB_OVERFLOW )) { +while (status >0 && (status & (IRB_FIFO_NOT_EMPTY |IRB_OVERFLOW ))) {u32 int_status =readl (dev->rx_base +IRB_RX_INT_STATUS );if (unlikely (int_status &IRB_RX_OVERRUN_INT )) {/* discard the entire collection in case of errors! */
Signature · 1 line
2.5.0
PATCH 4/6 st-rc: prevent a endless loop
Hi Mauro
Quoted 25 lines · Mauro Carvalho Chehab
As warned by smatch:
drivers/media/rc/st_rc.c:110st_rc_rx_interrupt()warn: this loop depends onreadl()succeedingas
readl()might fail, with likely means some unrecovered error, let's loop only if it succeeds.Signed-off-by Mauro Carvalho Chehab <[email protected]>--- drivers/media/rc/st_rc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/media/rc/st_rc.c +1
-1@@ -99,7 +99,7 @@ static irqreturn_t st_rc_rx_interrupt(int irq, void *data)unsigned int symbol, mark =0 ;struct st_rc_device *dev = data;int last_symbol =0 ; -u32 status; +int status;DEFINE_IR_RAW_EVENT (ev);if (dev->irq_wake)
drivers/media/rc/st_rc.c static irqreturn_t st_rc_rx_interrupt(int irq, void *data) · line 113
Quoted 8 lines
drivers/media/rc/st_rc.c +1
-1@@ -107,7 +107,7 @@ static irqreturn_t st_rc_rx_interrupt(int irq, void *data) status =readl (dev->rx_base +IRB_RX_STATUS ); -while (status & (IRB_FIFO_NOT_EMPTY |IRB_OVERFLOW )) { +while (status >0 && (status & (IRB_FIFO_NOT_EMPTY |IRB_OVERFLOW ))) {u32 int_status =readl (dev->rx_base +IRB_RX_INT_STATUS );if (unlikely (int_status &IRB_RX_OVERRUN_INT )) {/* discard the entire collection in case of errors! */
PATCH 5/6 touptek: don't DMA at the stack
In reply to Mauro Carvalho ChehabAs warned by smatch:
drivers/media/usb/gspca/touptek.c:220 reg_w() error: doing dma on the stack (buff) drivers/media/usb/gspca/touptek.c:458 configure() error: doing dma on the stack (buff)
This can fail, as the stack may not be in a memory that would allod DMA. So, use the usb_buf instead.
--- drivers/media/usb/gspca/touptek.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/media/usb/gspca/touptek.c +2 -2
@@ -211,7 +211,7 @@ static int val_reply(struct gspca_dev *gspca_dev, const char *reply, int rc)static void reg_w (struct gspca_dev *gspca_dev,u16 value,u16 index) { -char buff[1 ]; +char *buff = gspca_dev->usb_buf;int rc; PDEBUG(D_USBO,
@@ -438,7 +438,7 @@ static void configure_encrypted(struct gspca_dev *gspca_dev)static int configure (struct gspca_dev *gspca_dev) {int rc; -uint8_t buff[4 ]; +char *buff = gspca_dev->usb_buf;PDEBUG (D_STREAM ,"configure()\n" );
Signature · 1 line
2.5.0
PATCH 6/6 touptek: cast char types on %x printk
In reply to Mauro Carvalho ChehabThis fixes those two smatch warnings:
drivers/media/usb/gspca/touptek.c:206 val_reply() warn: argument 3 to %02x specifier has type 'char' drivers/media/usb/gspca/touptek.c:222 reg_w() warn: argument 4 to %02x specifier has type 'char'
--- drivers/media/usb/gspca/touptek.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/media/usb/gspca/touptek.c +2 -2
@@ -203,7 +203,7 @@ static int val_reply(struct gspca_dev *gspca_dev, const char *reply, int rc)return -EIO ; }if (reply[0 ] !=0x08 ) { -PERR ("Bad reply 0x%02X" , reply[0 ]); +PERR ("Bad reply 0x%02x" , (int )reply[0 ]);return -EIO ; }return 0 ;
@@ -219,7 +219,7 @@ static void reg_w(struct gspca_dev *gspca_dev, u16 value, u16 index) value, index); rc =usb_control_msg (gspca_dev->dev,usb_rcvctrlpipe (gspca_dev->dev,0 ),0x0B ,0xC0 , value, index, buff,1 ,500 ); -PDEBUG (D_USBO ,"rc=%d, ret={0x%02X}" , rc, buff[0 ]); +PDEBUG (D_USBO ,"rc=%d, ret={0x%02x}" , rc, (int )buff[0 ]);if (rc <0 ) {PERR ("Failed reg_w(0x0B, 0xC0, 0x%04X, 0x%04X) w/ rc %d\n" , value, index, rc);
Signature · 1 line
2.5.0