* [bug report] rsi: Add new host interface operations
@ 2017-06-12 19:59 Dan Carpenter
2017-07-07 14:28 ` Dan Carpenter
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2017-06-12 19:59 UTC (permalink / raw)
To: prameela.j04cs; +Cc: linux-wireless
Hello Prameela Rani Garnepudi,
The patch b97e9b94ad75: "rsi: Add new host interface operations" from
May 16, 2017, leads to the following static checker warning:
drivers/net/wireless/rsi/rsi_91x_usb.c:400 rsi_usb_master_reg_read()
warn: passing casted pointer 'value' to 'rsi_usb_reg_read()' 32 vs 16.
drivers/net/wireless/rsi/rsi_91x_usb.c
156 static int rsi_usb_reg_read(struct usb_device *usbdev,
157 u32 reg,
158 u16 *value,
159 u16 len)
160 {
161 u8 *buf;
162 int status = -ENOMEM;
163
164 buf = kmalloc(0x04, GFP_KERNEL);
Why are we allocating 0x4 bytes?
165 if (!buf)
166 return status;
167
168 status = usb_control_msg(usbdev,
169 usb_rcvctrlpipe(usbdev, 0),
170 USB_VENDOR_REGISTER_READ,
171 RSI_USB_REQ_IN,
172 ((reg & 0xffff0000) >> 16), (reg & 0xffff),
173 (void *)buf,
174 len,
^^^
If len is more than 4 then we have memory corruption.
175 USB_CTRL_GET_TIMEOUT);
176
177 *value = (buf[0] | (buf[1] << 8));
178 if (status < 0) {
179 rsi_dbg(ERR_ZONE,
180 "%s: Reg read failed with error code :%d\n",
181 __func__, status);
182 }
[ snip ]
394 static int rsi_usb_master_reg_read(struct rsi_hw *adapter, u32 reg,
395 u32 *value, u16 len)
396 {
397 struct usb_device *usbdev =
398 ((struct rsi_91x_usbdev *)adapter->rsi_dev)->usbdev;
399
400 return rsi_usb_reg_read(usbdev, reg, (u16 *)value, len);
This is an endian bug because we're assuming the CPU is little endian.
u16 tmp;
int ret;
if (len != sizeof(*value)) /* Or maybe sizeof(tmp), who knows? */
return -EINVAL;
ret = rsi_usb_reg_read(usbdev, reg, &tmp, len);
if (ret)
return ret;
*value = tmp;
return 0;
401 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [bug report] rsi: Add new host interface operations
2017-06-12 19:59 [bug report] rsi: Add new host interface operations Dan Carpenter
@ 2017-07-07 14:28 ` Dan Carpenter
2017-07-11 14:34 ` Amitkumar Karwar
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2017-07-07 14:28 UTC (permalink / raw)
To: prameela.j04cs; +Cc: linux-wireless, Amitkumar Karwar
None of these issues have been addressed.
regards,
dan carpenter
On Mon, Jun 12, 2017 at 10:59:08PM +0300, Dan Carpenter wrote:
> Hello Prameela Rani Garnepudi,
>
> The patch b97e9b94ad75: "rsi: Add new host interface operations" from
> May 16, 2017, leads to the following static checker warning:
>
> drivers/net/wireless/rsi/rsi_91x_usb.c:400 rsi_usb_master_reg_read()
> warn: passing casted pointer 'value' to 'rsi_usb_reg_read()' 32 vs 16.
>
> drivers/net/wireless/rsi/rsi_91x_usb.c
> 156 static int rsi_usb_reg_read(struct usb_device *usbdev,
> 157 u32 reg,
> 158 u16 *value,
> 159 u16 len)
> 160 {
> 161 u8 *buf;
> 162 int status = -ENOMEM;
> 163
> 164 buf = kmalloc(0x04, GFP_KERNEL);
>
> Why are we allocating 0x4 bytes?
>
> 165 if (!buf)
> 166 return status;
> 167
> 168 status = usb_control_msg(usbdev,
> 169 usb_rcvctrlpipe(usbdev, 0),
> 170 USB_VENDOR_REGISTER_READ,
> 171 RSI_USB_REQ_IN,
> 172 ((reg & 0xffff0000) >> 16), (reg & 0xffff),
> 173 (void *)buf,
> 174 len,
> ^^^
> If len is more than 4 then we have memory corruption.
>
> 175 USB_CTRL_GET_TIMEOUT);
> 176
> 177 *value = (buf[0] | (buf[1] << 8));
> 178 if (status < 0) {
> 179 rsi_dbg(ERR_ZONE,
> 180 "%s: Reg read failed with error code :%d\n",
> 181 __func__, status);
> 182 }
>
> [ snip ]
>
> 394 static int rsi_usb_master_reg_read(struct rsi_hw *adapter, u32 reg,
> 395 u32 *value, u16 len)
> 396 {
> 397 struct usb_device *usbdev =
> 398 ((struct rsi_91x_usbdev *)adapter->rsi_dev)->usbdev;
> 399
> 400 return rsi_usb_reg_read(usbdev, reg, (u16 *)value, len);
>
> This is an endian bug because we're assuming the CPU is little endian.
>
> u16 tmp;
> int ret;
>
> if (len != sizeof(*value)) /* Or maybe sizeof(tmp), who knows? */
> return -EINVAL;
> ret = rsi_usb_reg_read(usbdev, reg, &tmp, len);
> if (ret)
> return ret;
>
> *value = tmp;
> return 0;
>
>
> 401 }
>
> regards,
> dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [bug report] rsi: Add new host interface operations
2017-07-07 14:28 ` Dan Carpenter
@ 2017-07-11 14:34 ` Amitkumar Karwar
0 siblings, 0 replies; 3+ messages in thread
From: Amitkumar Karwar @ 2017-07-11 14:34 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Prameela Rani Garnepudi, linux-wireless, Amitkumar Karwar
Hi Dan,
On Fri, Jul 7, 2017 at 7:58 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> None of these issues have been addressed.
>
I have just submitted 3 patches which address these issues.
Thanks
Amitkumar
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-07-11 14:34 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-12 19:59 [bug report] rsi: Add new host interface operations Dan Carpenter
2017-07-07 14:28 ` Dan Carpenter
2017-07-11 14:34 ` Amitkumar Karwar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).