2014-05-31 13:10:31

by Malcolm Priestley

[permalink] [raw]
Subject: [PATCH] staging: vt6656: Fix vnt_rf_table_download __builtin_memcpy() addr* too small (3 vs 64).

Fix following errors
drivers/staging/vt6656/rf.c:1060 vnt_rf_table_download() error: __builtin_memcpy() 'addr2' too small (3 vs 64)
drivers/staging/vt6656/rf.c:1078 vnt_rf_table_download() error: __builtin_memcpy() 'addr3' too small (3 vs 64)
drivers/staging/vt6656/rf.c:1094 vnt_rf_table_download() error: __builtin_memcpy() 'addr1' too small (3 vs 48)
drivers/staging/vt6656/rf.c:1108 vnt_rf_table_download() error: __builtin_memcpy() 'addr2' too small (3 vs 64)

Reported-by: Dan Carpenter <[email protected]>
Cc: Dan Carpenter <[email protected]>
Cc: [email protected]
Signed-off-by: Malcolm Priestley <[email protected]>
---
drivers/staging/vt6656/rf.c | 34 +++++++++++++++++-----------------
1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/staging/vt6656/rf.c b/drivers/staging/vt6656/rf.c
index 3f54ae3..131764f 100644
--- a/drivers/staging/vt6656/rf.c
+++ b/drivers/staging/vt6656/rf.c
@@ -1003,41 +1003,41 @@ void vnt_rf_table_download(struct vnt_private *priv)
length1 = CB_AL2230_INIT_SEQ * 3;
length2 = CB_MAX_CHANNEL_24G * 3;
length3 = CB_MAX_CHANNEL_24G * 3;
- addr1 = &al2230_init_table[0][0];
- addr2 = &al2230_channel_table0[0][0];
- addr3 = &al2230_channel_table1[0][0];
+ addr1 = (u8 *)al2230_init_table;
+ addr2 = (u8 *)al2230_channel_table0;
+ addr3 = (u8 *)al2230_channel_table1;
break;
case RF_AIROHA7230:
length1 = CB_AL7230_INIT_SEQ * 3;
length2 = CB_MAX_CHANNEL * 3;
length3 = CB_MAX_CHANNEL * 3;
- addr1 = &al7230_init_table[0][0];
- addr2 = &al7230_channel_table0[0][0];
- addr3 = &al7230_channel_table1[0][0];
+ addr1 = (u8 *)al7230_init_table;
+ addr2 = (u8 *)al7230_channel_table0;
+ addr3 = (u8 *)al7230_channel_table1;
break;
case RF_VT3226:
length1 = CB_VT3226_INIT_SEQ * 3;
length2 = CB_MAX_CHANNEL_24G * 3;
length3 = CB_MAX_CHANNEL_24G * 3;
- addr1 = &vt3226_init_table[0][0];
- addr2 = &vt3226_channel_table0[0][0];
- addr3 = &vt3226_channel_table1[0][0];
+ addr1 = (u8 *)vt3226_init_table;
+ addr2 = (u8 *)vt3226_channel_table0;
+ addr3 = (u8 *)vt3226_channel_table1;
break;
case RF_VT3226D0:
length1 = CB_VT3226_INIT_SEQ * 3;
length2 = CB_MAX_CHANNEL_24G * 3;
length3 = CB_MAX_CHANNEL_24G * 3;
- addr1 = &vt3226d0_init_table[0][0];
- addr2 = &vt3226_channel_table0[0][0];
- addr3 = &vt3226_channel_table1[0][0];
+ addr1 = (u8 *)vt3226d0_init_table;
+ addr2 = (u8 *)vt3226_channel_table0;
+ addr3 = (u8 *)vt3226_channel_table1;
break;
case RF_VT3342A0:
length1 = CB_VT3342_INIT_SEQ * 3;
length2 = CB_MAX_CHANNEL * 3;
length3 = CB_MAX_CHANNEL * 3;
- addr1 = &vt3342a0_init_table[0][0];
- addr2 = &vt3342_channel_table0[0][0];
- addr3 = &vt3342_channel_table1[0][0];
+ addr1 = (u8 *)vt3342a0_init_table;
+ addr2 = (u8 *)vt3342_channel_table0;
+ addr3 = (u8 *)vt3342_channel_table1;
break;
}

@@ -1086,8 +1086,8 @@ void vnt_rf_table_download(struct vnt_private *priv)
if (priv->byRFType == RF_AIROHA7230) {
length1 = CB_AL7230_INIT_SEQ * 3;
length2 = CB_MAX_CHANNEL * 3;
- addr1 = &(al7230_init_table_amode[0][0]);
- addr2 = &(al7230_channel_table2[0][0]);
+ addr1 = (u8 *)al7230_init_table_amode;
+ addr2 = (u8 *)al7230_channel_table2;

memcpy(array, addr1, length1);

--
1.9.1



2014-05-31 14:57:29

by Malcolm Priestley

[permalink] [raw]
Subject: Re: [kbuild] [PATCH] staging: vt6656: Fix vnt_rf_table_download __builtin_memcpy() addr* too small (3 vs 64).

On 31/05/14 15:11, Dan Carpenter wrote:
> On Sat, May 31, 2014 at 04:42:02PM +0300, Dan Carpenter wrote:
>> On Sat, May 31, 2014 at 02:09:27PM +0100, Malcolm Priestley wrote:
>>> Fix following errors
>>> drivers/staging/vt6656/rf.c:1060 vnt_rf_table_download() error: __builtin_memcpy() 'addr2' too small (3 vs 64)
>>> drivers/staging/vt6656/rf.c:1078 vnt_rf_table_download() error: __builtin_memcpy() 'addr3' too small (3 vs 64)
>>> drivers/staging/vt6656/rf.c:1094 vnt_rf_table_download() error: __builtin_memcpy() 'addr1' too small (3 vs 48)
>>> drivers/staging/vt6656/rf.c:1108 vnt_rf_table_download() error: __builtin_memcpy() 'addr2' too small (3 vs 64)
>>>
>>
>
> Btw, the 3 here is a bug in Smatch... I will fix that. You shouldn't
> have to work around that like this. I'm sorry for the confusion. My
> only question when I reported the Smatch warning was about where we got
> the 64.
>
64 is the maximum that can be sent out at any one time so it must be
from length.

array could be reduced to 64.

3 is the element size of the tables, so the pointer should really point
to the whole table.

Regards


Malcolm







2014-05-31 14:12:01

by Dan Carpenter

[permalink] [raw]
Subject: Re: [kbuild] [PATCH] staging: vt6656: Fix vnt_rf_table_download __builtin_memcpy() addr* too small (3 vs 64).

On Sat, May 31, 2014 at 04:42:02PM +0300, Dan Carpenter wrote:
> On Sat, May 31, 2014 at 02:09:27PM +0100, Malcolm Priestley wrote:
> > Fix following errors
> > drivers/staging/vt6656/rf.c:1060 vnt_rf_table_download() error: __builtin_memcpy() 'addr2' too small (3 vs 64)
> > drivers/staging/vt6656/rf.c:1078 vnt_rf_table_download() error: __builtin_memcpy() 'addr3' too small (3 vs 64)
> > drivers/staging/vt6656/rf.c:1094 vnt_rf_table_download() error: __builtin_memcpy() 'addr1' too small (3 vs 48)
> > drivers/staging/vt6656/rf.c:1108 vnt_rf_table_download() error: __builtin_memcpy() 'addr2' too small (3 vs 64)
> >
>

Btw, the 3 here is a bug in Smatch... I will fix that. You shouldn't
have to work around that like this. I'm sorry for the confusion. My
only question when I reported the Smatch warning was about where we got
the 64.

regards,
dan carpenter


2014-05-31 13:42:18

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH] staging: vt6656: Fix vnt_rf_table_download __builtin_memcpy() addr* too small (3 vs 64).

On Sat, May 31, 2014 at 02:09:27PM +0100, Malcolm Priestley wrote:
> Fix following errors
> drivers/staging/vt6656/rf.c:1060 vnt_rf_table_download() error: __builtin_memcpy() 'addr2' too small (3 vs 64)
> drivers/staging/vt6656/rf.c:1078 vnt_rf_table_download() error: __builtin_memcpy() 'addr3' too small (3 vs 64)
> drivers/staging/vt6656/rf.c:1094 vnt_rf_table_download() error: __builtin_memcpy() 'addr1' too small (3 vs 48)
> drivers/staging/vt6656/rf.c:1108 vnt_rf_table_download() error: __builtin_memcpy() 'addr2' too small (3 vs 64)
>

I guess I was confused where the 64 comes from in the:

if (length2 >= 64)
length = 64;

We only ever seem to use the first 64 bytes of "u8 array[256];" so my
guess is that it was intended to be sizeof(array)?

regards,
dan carpenter


2014-06-19 22:55:30

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [kbuild] [PATCH] staging: vt6656: Fix vnt_rf_table_download __builtin_memcpy() addr* too small (3 vs 64).

On Sat, May 31, 2014 at 03:57:24PM +0100, Malcolm Priestley wrote:
> On 31/05/14 15:11, Dan Carpenter wrote:
> >On Sat, May 31, 2014 at 04:42:02PM +0300, Dan Carpenter wrote:
> >>On Sat, May 31, 2014 at 02:09:27PM +0100, Malcolm Priestley wrote:
> >>>Fix following errors
> >>>drivers/staging/vt6656/rf.c:1060 vnt_rf_table_download() error: __builtin_memcpy() 'addr2' too small (3 vs 64)
> >>>drivers/staging/vt6656/rf.c:1078 vnt_rf_table_download() error: __builtin_memcpy() 'addr3' too small (3 vs 64)
> >>>drivers/staging/vt6656/rf.c:1094 vnt_rf_table_download() error: __builtin_memcpy() 'addr1' too small (3 vs 48)
> >>>drivers/staging/vt6656/rf.c:1108 vnt_rf_table_download() error: __builtin_memcpy() 'addr2' too small (3 vs 64)
> >>>
> >>
> >
> >Btw, the 3 here is a bug in Smatch... I will fix that. You shouldn't
> >have to work around that like this. I'm sorry for the confusion. My
> >only question when I reported the Smatch warning was about where we got
> >the 64.
> >
> 64 is the maximum that can be sent out at any one time so it must be from
> length.
>
> array could be reduced to 64.
>
> 3 is the element size of the tables, so the pointer should really point to
> the whole table.

I'm dropping this because it shouldn't be needed, right? If not, please
resend.

thanks,

greg k-h