From 365741e6a6ab7f31ff885c26dd173d9590d1e8ca Mon Sep 17 00:00:00 2001 From: H Hartley Sweeten Date: Mon, 10 Aug 2015 10:28:08 -0700 Subject: [PATCH] staging: comedi: ni_usb6501: fix possible out-of-bounds access Coverity reports a possible Out-of-bounds access (ARRAY_VS_SINGLETON) with the 'const u8 *port' parameter passed to ni6501_port_command(). This param is an actual array for the SET_PORT_DIR operation, called by ni6501_dio_insn_config(). But for the WRITE_PORT and READ_PORT operations, called by ni6501_dio_insn_bits(), it is just the address of an u8 local variable. Fix the coverity issue by changing the parameter to an unsigned int and pass the raw values from ni6501_dio_insn_config() and ni6501_dio_insn_bits(). ni6501_port_command() then handles the masking and shifting needed to load the value into the u8 transmit buffer. For consistency, change the access of the 'bitmap' parameter from an array access to a pointer operation. Reported-by: coverity (CID 1248624) Signed-off-by: H Hartley Sweeten Reviewed-by: Ian Abbott Signed-off-by: Greg Kroah-Hartman --- drivers/staging/comedi/drivers/ni_usb6501.c | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/drivers/staging/comedi/drivers/ni_usb6501.c b/drivers/staging/comedi/drivers/ni_usb6501.c index 5f649f8..88de8da 100644 --- a/drivers/staging/comedi/drivers/ni_usb6501.c +++ b/drivers/staging/comedi/drivers/ni_usb6501.c @@ -172,7 +172,7 @@ struct ni6501_private { }; static int ni6501_port_command(struct comedi_device *dev, int command, - const u8 *port, u8 *bitmap) + unsigned int val, u8 *bitmap) { struct usb_device *usb = comedi_to_usb_dev(dev); struct ni6501_private *devpriv = dev->private; @@ -190,22 +190,22 @@ static int ni6501_port_command(struct comedi_device *dev, int command, request_size = sizeof(READ_PORT_REQUEST); response_size = sizeof(READ_PORT_RESPONSE); memcpy(tx, READ_PORT_REQUEST, request_size); - tx[14] = port[0]; + tx[14] = val & 0xff; break; case WRITE_PORT: request_size = sizeof(WRITE_PORT_REQUEST); response_size = sizeof(GENERIC_RESPONSE); memcpy(tx, WRITE_PORT_REQUEST, request_size); - tx[14] = port[0]; - tx[17] = bitmap[0]; + tx[14] = val & 0xff; + tx[17] = *bitmap; break; case SET_PORT_DIR: request_size = sizeof(SET_PORT_DIR_REQUEST); response_size = sizeof(GENERIC_RESPONSE); memcpy(tx, SET_PORT_DIR_REQUEST, request_size); - tx[14] = port[0]; - tx[15] = port[1]; - tx[16] = port[2]; + tx[14] = val & 0xff; + tx[15] = (val >> 8) & 0xff; + tx[16] = (val >> 16) & 0xff; break; default: ret = -EINVAL; @@ -235,7 +235,7 @@ static int ni6501_port_command(struct comedi_device *dev, int command, /* Check if results are valid */ if (command == READ_PORT) { - bitmap[0] = devpriv->usb_rx_buf[14]; + *bitmap = devpriv->usb_rx_buf[14]; /* mask bitmap for comparing */ devpriv->usb_rx_buf[14] = 0x00; @@ -349,17 +349,12 @@ static int ni6501_dio_insn_config(struct comedi_device *dev, unsigned int *data) { int ret; - u8 port[3]; ret = comedi_dio_insn_config(dev, s, insn, data, 0); if (ret) return ret; - port[0] = (s->io_bits) & 0xff; - port[1] = (s->io_bits >> 8) & 0xff; - port[2] = (s->io_bits >> 16) & 0xff; - - ret = ni6501_port_command(dev, SET_PORT_DIR, port, NULL); + ret = ni6501_port_command(dev, SET_PORT_DIR, s->io_bits, NULL); if (ret) return ret; @@ -382,7 +377,7 @@ static int ni6501_dio_insn_bits(struct comedi_device *dev, if (mask & (0xFF << port * 8)) { bitmap = (s->state >> port * 8) & 0xFF; ret = ni6501_port_command(dev, WRITE_PORT, - &port, &bitmap); + port, &bitmap); if (ret) return ret; } @@ -391,7 +386,7 @@ static int ni6501_dio_insn_bits(struct comedi_device *dev, data[1] = 0; for (port = 0; port < 3; port++) { - ret = ni6501_port_command(dev, READ_PORT, &port, &bitmap); + ret = ni6501_port_command(dev, READ_PORT, port, &bitmap); if (ret) return ret; data[1] |= bitmap << port * 8; -- 2.7.4