[PATCH -next] [media] go7007: fix invalid use of sizeof in go7007_usb_i2c_master_xfer()

[PATCH -next] [media] go7007: fix invalid use of sizeof in go7007_usb_i2c_master_xfer()

[Wei Yongjun]

From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>

sizeof() when applied to a pointer typed expression gives the
size of the pointer, not that of the pointed data.

Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
---
 drivers/staging/media/go7007/go7007-usb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/go7007/go7007-usb.c b/drivers/staging/media/go7007/go7007-usb.c
index 0823506..7219ae0 100644
--- a/drivers/staging/media/go7007/go7007-usb.c
+++ b/drivers/staging/media/go7007/go7007-usb.c
@@ -1035,7 +1035,7 @@ static int go7007_usb_i2c_master_xfer(struct i2c_adapter *adapter,
 						buf, buf_len, 0) < 0)
 			goto i2c_done;
 		if (msgs[i].flags & I2C_M_RD) {
-			memset(buf, 0, sizeof(buf));
+			memset(buf, 0, sizeof(*buf));
 			if (go7007_usb_vendor_request(go, 0x25, 0, 0, buf,
 						msgs[i].len + 1, 1) < 0)
 				goto i2c_done;

Re: [PATCH -next] [media] go7007: fix invalid use of sizeof in go7007_usb_i2c_master_xfer()

[Dan Carpenter]

On Tue, Mar 26, 2013 at 02:42:47PM +0800, Wei Yongjun wrote:
> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> 
> sizeof() when applied to a pointer typed expression gives the
> size of the pointer, not that of the pointed data.
>

This fix isn't right.  "buf" is a char pointer.  I don't know what
this code is doing.  Instead of sizeof(*buf) it should be something
like "buflen", "msg[i].len", "msg[i].len + 1" or "msg[i].len + 3".

I'm not sure which is correct here or what it's doing, sorry.

regards,
dan carpenter

Re: [PATCH -next] [media] go7007: fix invalid use of sizeof in go7007_usb_i2c_master_xfer()

[Dan Carpenter]

On Tue, Mar 26, 2013 at 10:04:15AM +0300, Dan Carpenter wrote:
> On Tue, Mar 26, 2013 at 02:42:47PM +0800, Wei Yongjun wrote:
> > From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> > 
> > sizeof() when applied to a pointer typed expression gives the
> > size of the pointer, not that of the pointed data.
> >
> 
> This fix isn't right.  "buf" is a char pointer.  I don't know what
> this code is doing.  Instead of sizeof(*buf) it should be something
> like "buflen", "msg[i].len", "msg[i].len + 1" or "msg[i].len + 3".

It should be "msg[i].len + 1", I think.

On the line before it writes buflen bytes to the hardware.  Then
it clears the transfer buffer and reads "msg[i].len + 1" bytes from
the hardware.  Then it saves the memory, except for the first byte,
in msg[i].buf.

So it should clear "msg[i].len + 1" bytes so that the old data isn't
confused with the data that we read from the hardware.

regards,
dan carpenter

Re: [PATCH -next] [media] go7007: fix invalid use of sizeof in go7007_usb_i2c_master_xfer()

[Hans Verkuil]

On Tue March 26 2013 08:35:57 Dan Carpenter wrote:
> On Tue, Mar 26, 2013 at 10:04:15AM +0300, Dan Carpenter wrote:
> > On Tue, Mar 26, 2013 at 02:42:47PM +0800, Wei Yongjun wrote:
> > > From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> > > 
> > > sizeof() when applied to a pointer typed expression gives the
> > > size of the pointer, not that of the pointed data.
> > >
> > 
> > This fix isn't right.  "buf" is a char pointer.  I don't know what
> > this code is doing.  Instead of sizeof(*buf) it should be something
> > like "buflen", "msg[i].len", "msg[i].len + 1" or "msg[i].len + 3".
> 
> It should be "msg[i].len + 1", I think.

Yes, that's correct.

'buf' used to be a local array, so the memset was fine. I changed it to an
array that was kmalloc()ed but forgot about the memset. I never noticed
the bug because the sizeof the message is typically quite small, certainly
smaller than sizeof(pointer) on a 64-bit system.

Wei Yongjun, can you post a new patch fixing this?

Thanks,

	Hans

> 
> On the line before it writes buflen bytes to the hardware.  Then
> it clears the transfer buffer and reads "msg[i].len + 1" bytes from
> the hardware.  Then it saves the memory, except for the first byte,
> in msg[i].buf.
> 
> So it should clear "msg[i].len + 1" bytes so that the old data isn't
> confused with the data that we read from the hardware.
> 
> regards,
> dan carpenter
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH -next] [media] go7007: fix invalid use of sizeof in go7007_usb_i2c_master_xfer()

[Wei Yongjun]

Hi Hans and Dan Carpenter,

On 03/26/2013 04:18 PM, Hans Verkuil wrote:
> On Tue March 26 2013 08:35:57 Dan Carpenter wrote:
>> On Tue, Mar 26, 2013 at 10:04:15AM +0300, Dan Carpenter wrote:
>>> On Tue, Mar 26, 2013 at 02:42:47PM +0800, Wei Yongjun wrote:
>>>> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
>>>>
>>>> sizeof() when applied to a pointer typed expression gives the
>>>> size of the pointer, not that of the pointed data.
>>>>
>>> This fix isn't right.  "buf" is a char pointer.  I don't know what
>>> this code is doing.  Instead of sizeof(*buf) it should be something
>>> like "buflen", "msg[i].len", "msg[i].len + 1" or "msg[i].len + 3".
>> It should be "msg[i].len + 1", I think.
> Yes, that's correct.
>
> 'buf' used to be a local array, so the memset was fine. I changed it to an
> array that was kmalloc()ed but forgot about the memset. I never noticed
> the bug because the sizeof the message is typically quite small, certainly
> smaller than sizeof(pointer) on a 64-bit system.
>
> Wei Yongjun, can you post a new patch fixing this?

Thanks very much, I will send the v2 of this patch soon.

Regards,
Yongjun