2013-09-20 09:50:47

by Thomas Meyer

[permalink] [raw]
Subject: [PATCH 0/10] Cocci spatch "noderef" - v3.11-7547-g44598f9

sizeof when applied to a pointer typed expression gives the size of the
pointer.
Found by coccinelle spatch "misc/noderef.cocci"

Run against version v3.11-7547-g44598f9

Let me know when you as a maintainer are not interested in these kind of patches.
I can exclude you by path; all cocci findings in e.g. "drivers/scsi" will never
be reported again by this semi-automatic cocci program runs.


2013-09-19 21:08:19

by Hauke Mehrtens

[permalink] [raw]
Subject: Re: [PATCH 1/10] MIPS: BCM47XX: Cocci spatch "noderef"

On 09/19/2013 08:38 PM, [email protected] wrote:
> sizeof when applied to a pointer typed expression gives the size of the
> pointer.
> Found by coccinelle spatch "misc/noderef.cocci"

Thanks for spotting this.

Is this a new rule or has just nobody checked that part of the kernel?


The from field in the mail is broken.

> Signed-off-by: Thomas Meyer <[email protected]>
Acked-by: Hauke Mehrtens <[email protected]>

> diff -u -p a/arch/mips/bcm47xx/sprom.c b/arch/mips/bcm47xx/sprom.c
> --- a/arch/mips/bcm47xx/sprom.c
> +++ b/arch/mips/bcm47xx/sprom.c
> @@ -162,7 +162,7 @@ static void nvram_read_alpha2(const char
> pr_warn("alpha2 is too long %s\n", buf);
> return;
> }
> - memcpy(val, buf, sizeof(val));
> + memcpy(val, buf, sizeof(*val));
> }
>
> static void bcm47xx_fill_sprom_r1234589(struct ssb_sprom *sprom,
>

2013-09-19 21:44:28

by Thomas Meyer

[permalink] [raw]
Subject: [PATCH 3/10] iio: at91_adc: Cocci spatch "noderef"

sizeof when applied to a pointer typed expression gives the size of the
pointer.
Found by coccinelle spatch "misc/noderef.cocci"

Signed-off-by: Thomas Meyer <[email protected]>
---

diff -u -p a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
--- a/drivers/iio/adc/at91_adc.c
+++ b/drivers/iio/adc/at91_adc.c
@@ -279,7 +279,7 @@ static int at91_adc_trigger_init(struct
int i, ret;

st->trig = devm_kzalloc(&idev->dev,
- st->trigger_number * sizeof(st->trig),
+ st->trigger_number * sizeof(*st->trig),
GFP_KERNEL);

if (st->trig == NULL) {

2013-09-19 21:44:31

by Thomas Meyer

[permalink] [raw]
Subject: [PATCH 2/10] xtensa: Cocci spatch "noderef"

sizeof when applied to a pointer typed expression gives the size of the
pointer.
Found by coccinelle spatch "misc/noderef.cocci"

Signed-off-by: Thomas Meyer <[email protected]>
---

diff -u -p a/arch/xtensa/platforms/iss/network.c b/arch/xtensa/platforms/iss/network.c
--- a/arch/xtensa/platforms/iss/network.c
+++ b/arch/xtensa/platforms/iss/network.c
@@ -737,7 +737,7 @@ static int __init iss_net_setup(char *st
return 1;
}

- if ((new = alloc_bootmem(sizeof new)) == NULL) {
+ if ((new = alloc_bootmem(sizeof *new)) == NULL) {
printk("Alloc_bootmem failed\n");
return 1;
}

2013-09-19 22:01:24

by Thomas Meyer

[permalink] [raw]
Subject: [PATCH 6/10] staging: octeon-usb: Cocci spatch "noderef"

sizeof when applied to a pointer typed expression gives the size of the
pointer.
Found by coccinelle spatch "misc/noderef.cocci"

Signed-off-by: Thomas Meyer <[email protected]>
---

diff -u -p a/drivers/staging/octeon-usb/cvmx-usb.c b/drivers/staging/octeon-usb/cvmx-usb.c
--- a/drivers/staging/octeon-usb/cvmx-usb.c
+++ b/drivers/staging/octeon-usb/cvmx-usb.c
@@ -604,7 +604,7 @@ int cvmx_usb_initialize(struct cvmx_usb_
}
}

- memset(usb, 0, sizeof(usb));
+ memset(usb, 0, sizeof(*usb));
usb->init_flags = flags;

/* Initialize the USB state structure */

2013-09-19 22:01:39

by Thomas Meyer

[permalink] [raw]
Subject: [PATCH 9/10] staging: r8188eu: Cocci spatch "noderef"

sizeof when applied to a pointer typed expression gives the size of the
pointer.
Found by coccinelle spatch "misc/noderef.cocci"

Signed-off-by: Thomas Meyer <[email protected]>
---

diff -u -p a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
--- a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
+++ b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
@@ -6973,7 +6973,7 @@ static int rtw_mp_ctx(struct net_device
stop = strncmp(extra, "stop", 4);
sscanf(extra, "count =%d, pkt", &count);

- _rtw_memset(extra, '\0', sizeof(extra));
+ _rtw_memset(extra, '\0', sizeof(*extra));

if (stop == 0) {
bStartTest = 0; /* To set Stop */

2013-09-19 22:01:43

by Thomas Meyer

[permalink] [raw]
Subject: [PATCH 8/10] staging: r8188eu: Add files for new drive: Cocci spatch "noderef"

sizeof when applied to a pointer typed expression gives the size of the
pointer.
Found by coccinelle spatch "misc/noderef.cocci"

Signed-off-by: Thomas Meyer <[email protected]>
---

diff -u -p a/drivers/staging/rtl8188eu/hal/rtl8188e_dm.c b/drivers/staging/rtl8188eu/hal/rtl8188e_dm.c
--- a/drivers/staging/rtl8188eu/hal/rtl8188e_dm.c
+++ b/drivers/staging/rtl8188eu/hal/rtl8188e_dm.c
@@ -57,7 +57,7 @@ static void Init_ODM_ComInfo_88E(struct
u8 cut_ver, fab_ver;

/* Init Value */
- _rtw_memset(dm_odm, 0, sizeof(dm_odm));
+ _rtw_memset(dm_odm, 0, sizeof(*dm_odm));

dm_odm->Adapter = Adapter;

2013-09-19 22:02:05

by Thomas Meyer

[permalink] [raw]
Subject: [PATCH 5/10] staging: lustre: Cocci spatch "noderef"

sizeof when applied to a pointer typed expression gives the size of the
pointer.
Found by coccinelle spatch "misc/noderef.cocci"

Signed-off-by: Thomas Meyer <[email protected]>
---

diff -u -p a/drivers/staging/lustre/lustre/obdecho/echo_client.c b/drivers/staging/lustre/lustre/obdecho/echo_client.c
--- a/drivers/staging/lustre/lustre/obdecho/echo_client.c
+++ b/drivers/staging/lustre/lustre/obdecho/echo_client.c
@@ -1387,7 +1387,7 @@ echo_copyout_lsm (struct lov_stripe_md *
if (nob > ulsm_nob)
return (-EINVAL);

- if (copy_to_user (ulsm, lsm, sizeof(ulsm)))
+ if (copy_to_user (ulsm, lsm, sizeof(*ulsm)))
return (-EFAULT);

for (i = 0; i < lsm->lsm_stripe_count; i++) {

2013-09-19 22:02:45

by Thomas Meyer

[permalink] [raw]
Subject: [PATCH 4/10] [SCSI] csiostor: Cocci spatch "noderef"

sizeof when applied to a pointer typed expression gives the size of the
pointer.
Found by coccinelle spatch "misc/noderef.cocci"

Signed-off-by: Thomas Meyer <[email protected]>
---

diff -u -p a/drivers/scsi/csiostor/csio_mb.c b/drivers/scsi/csiostor/csio_mb.c
--- a/drivers/scsi/csiostor/csio_mb.c
+++ b/drivers/scsi/csiostor/csio_mb.c
@@ -1531,7 +1531,7 @@ csio_mb_isr_handler(struct csio_hw *hw)
* Enqueue event to EventQ. Events processing happens
* in Event worker thread context
*/
- if (csio_enqueue_evt(hw, CSIO_EVT_MBX, mbp, sizeof(mbp)))
+ if (csio_enqueue_evt(hw, CSIO_EVT_MBX, mbp, sizeof(*mbp)))
CSIO_INC_STATS(hw, n_evt_drop);

return 0;

2013-09-19 22:01:21

by Thomas Meyer

[permalink] [raw]
Subject: [PATCH 7/10] staging: r8188eu: Add files for new drive: Cocci spatch "noderef"

sizeof when applied to a pointer typed expression gives the size of the
pointer.
Found by coccinelle spatch "misc/noderef.cocci"

Signed-off-by: Thomas Meyer <[email protected]>
---

diff -u -p a/drivers/staging/rtl8188eu/core/rtw_mp.c b/drivers/staging/rtl8188eu/core/rtw_mp.c
--- a/drivers/staging/rtl8188eu/core/rtw_mp.c
+++ b/drivers/staging/rtl8188eu/core/rtw_mp.c
@@ -907,7 +907,7 @@ u32 mp_query_psd(struct adapter *pAdapte
sscanf(data, "pts =%d, start =%d, stop =%d", &psd_pts, &psd_start, &psd_stop);
}

- _rtw_memset(data, '\0', sizeof(data));
+ _rtw_memset(data, '\0', sizeof(*data));

i = psd_start;
while (i < psd_stop) {

2013-09-19 22:10:27

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH 2/10] xtensa: Cocci spatch "noderef"

On Thu, 2013-09-19 at 23:42 +0200, Thomas Meyer wrote:
> sizeof when applied to a pointer typed expression gives the size of the
> pointer.

Hi Thomas, thanks for doing the series...

> diff -u -p a/arch/xtensa/platforms/iss/network.c b/arch/xtensa/platforms/iss/network.c
[]
> @@ -737,7 +737,7 @@ static int __init iss_net_setup(char *st
> return 1;
> }
>
> - if ((new = alloc_bootmem(sizeof new)) == NULL) {
> + if ((new = alloc_bootmem(sizeof *new)) == NULL) {

sizeof(*new)

Please run coccinelle generated patches through checkpatch
(and the compiler of course) before submitting them.

2013-09-20 07:26:27

by Nicolas Ferre

[permalink] [raw]
Subject: Re: [PATCH 3/10] iio: at91_adc: Cocci spatch "noderef"

On 19/09/2013 23:42, Thomas Meyer :
> sizeof when applied to a pointer typed expression gives the size of the
> pointer.
> Found by coccinelle spatch "misc/noderef.cocci"
>
> Signed-off-by: Thomas Meyer <[email protected]>

Acked-by: Nicolas Ferre <[email protected]>

> ---
>
> diff -u -p a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
> --- a/drivers/iio/adc/at91_adc.c
> +++ b/drivers/iio/adc/at91_adc.c
> @@ -279,7 +279,7 @@ static int at91_adc_trigger_init(struct
> int i, ret;
>
> st->trig = devm_kzalloc(&idev->dev,
> - st->trigger_number * sizeof(st->trig),
> + st->trigger_number * sizeof(*st->trig),
> GFP_KERNEL);
>
> if (st->trig == NULL) {
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>


--
Nicolas Ferre

2013-09-20 09:50:48

by Thomas Meyer

[permalink] [raw]
Subject: [PATCH 1/10] MIPS: BCM47XX: Cocci spatch "noderef"

sizeof when applied to a pointer typed expression gives the size of the
pointer.
Found by coccinelle spatch "misc/noderef.cocci"

Signed-off-by: Thomas Meyer <[email protected]>
---

diff -u -p a/arch/mips/bcm47xx/sprom.c b/arch/mips/bcm47xx/sprom.c
--- a/arch/mips/bcm47xx/sprom.c
+++ b/arch/mips/bcm47xx/sprom.c
@@ -162,7 +162,7 @@ static void nvram_read_alpha2(const char
pr_warn("alpha2 is too long %s\n", buf);
return;
}
- memcpy(val, buf, sizeof(val));
+ memcpy(val, buf, sizeof(*val));
}

static void bcm47xx_fill_sprom_r1234589(struct ssb_sprom *sprom,

2013-09-20 10:44:09

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH 7/10] staging: r8188eu: Add files for new drive: Cocci spatch "noderef"

On Thu, Sep 19, 2013 at 11:45:46PM +0200, Thomas Meyer wrote:
> sizeof when applied to a pointer typed expression gives the size of the
> pointer.
> Found by coccinelle spatch "misc/noderef.cocci"
>

When you're writing the changelog for these it helps if you say how
many bytes sizeof(*data) is. In this case, we have gone from clearing 8
bytes to clearing 1 byte so the original code had a potential memory
corruption bug.

> Signed-off-by: Thomas Meyer <[email protected]>
> ---
>
> diff -u -p a/drivers/staging/rtl8188eu/core/rtw_mp.c b/drivers/staging/rtl8188eu/core/rtw_mp.c
> --- a/drivers/staging/rtl8188eu/core/rtw_mp.c
> +++ b/drivers/staging/rtl8188eu/core/rtw_mp.c
> @@ -907,7 +907,7 @@ u32 mp_query_psd(struct adapter *pAdapte
> sscanf(data, "pts =%d, start =%d, stop =%d", &psd_pts, &psd_start, &psd_stop);
> }
>
> - _rtw_memset(data, '\0', sizeof(data));
> + _rtw_memset(data, '\0', sizeof(*data));

I think your fix is correct but it would be better to remove the memset
and do:

data[0] = '\0';

"data" is a u8 pointer, but it should obviously be a char pointer. The
original code here is not high quality. :P

regards,
dan carpenter

2013-09-20 10:45:20

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH 9/10] staging: r8188eu: Cocci spatch "noderef"

On Thu, Sep 19, 2013 at 11:45:46PM +0200, Thomas Meyer wrote:
> sizeof when applied to a pointer typed expression gives the size of the
> pointer.
> Found by coccinelle spatch "misc/noderef.cocci"
>
> Signed-off-by: Thomas Meyer <[email protected]>
> ---
>
> diff -u -p a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
> --- a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
> +++ b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
> @@ -6973,7 +6973,7 @@ static int rtw_mp_ctx(struct net_device
> stop = strncmp(extra, "stop", 4);
> sscanf(extra, "count =%d, pkt", &count);
>
> - _rtw_memset(extra, '\0', sizeof(extra));
> + _rtw_memset(extra, '\0', sizeof(*extra));

Do:

extra[0] = '\0';

regards,
dan carpenter

2013-09-20 17:36:17

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH 3/10] iio: at91_adc: Cocci spatch "noderef"

On 09/20/13 08:26, Nicolas Ferre wrote:
> On 19/09/2013 23:42, Thomas Meyer :
>> sizeof when applied to a pointer typed expression gives the size of the
>> pointer.
>> Found by coccinelle spatch "misc/noderef.cocci"
>>
>> Signed-off-by: Thomas Meyer <[email protected]>
>
> Acked-by: Nicolas Ferre <[email protected]>
Applied to the fixes-togreg branch of
git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git

Thanks,
>
>> ---
>>
>> diff -u -p a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
>> --- a/drivers/iio/adc/at91_adc.c
>> +++ b/drivers/iio/adc/at91_adc.c
>> @@ -279,7 +279,7 @@ static int at91_adc_trigger_init(struct
>> int i, ret;
>>
>> st->trig = devm_kzalloc(&idev->dev,
>> - st->trigger_number * sizeof(st->trig),
>> + st->trigger_number * sizeof(*st->trig),
>> GFP_KERNEL);
>>
>> if (st->trig == NULL) {
>> --
>> To unsubscribe from this list: send the line "unsubscribe devicetree" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>
>

2013-09-20 17:42:22

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH 3/10] iio: at91_adc: Cocci spatch "noderef"

On 09/20/13 19:36, Jonathan Cameron wrote:
> On 09/20/13 08:26, Nicolas Ferre wrote:
>> On 19/09/2013 23:42, Thomas Meyer :
>>> sizeof when applied to a pointer typed expression gives the size of the
>>> pointer.
>>> Found by coccinelle spatch "misc/noderef.cocci"
>>>
>>> Signed-off-by: Thomas Meyer <[email protected]>
>>
>> Acked-by: Nicolas Ferre <[email protected]>
> Applied to the fixes-togreg branch of
> git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git

Actually, change of plan. Applied to the togreg branch of that
tree. Whilst technically more correct to have it as you say,
we are dealing with the size of a struct iio_trig ** vs
a struct iio_trig * so it isn't actually a bug, just a
less than ideal bit of code ;)

It was so obviously a fix I didn't initially check if it
was a 'real' bug or not. oops.

Jonathan


>
> Thanks,
>>
>>> ---
>>>
>>> diff -u -p a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
>>> --- a/drivers/iio/adc/at91_adc.c
>>> +++ b/drivers/iio/adc/at91_adc.c
>>> @@ -279,7 +279,7 @@ static int at91_adc_trigger_init(struct
>>> int i, ret;
>>>
>>> st->trig = devm_kzalloc(&idev->dev,
>>> - st->trigger_number * sizeof(st->trig),
>>> + st->trigger_number * sizeof(*st->trig),
>>> GFP_KERNEL);
>>>
>>> if (st->trig == NULL) {
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe devicetree" in
>>> the body of a message to [email protected]
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>
>>
>>

2013-09-23 12:16:58

by Nicolas Ferre

[permalink] [raw]
Subject: Re: [PATCH 3/10] iio: at91_adc: Cocci spatch "noderef"

On 20/09/2013 20:42, Jonathan Cameron :
> On 09/20/13 19:36, Jonathan Cameron wrote:
>> On 09/20/13 08:26, Nicolas Ferre wrote:
>>> On 19/09/2013 23:42, Thomas Meyer :
>>>> sizeof when applied to a pointer typed expression gives the size of the
>>>> pointer.
>>>> Found by coccinelle spatch "misc/noderef.cocci"
>>>>
>>>> Signed-off-by: Thomas Meyer <[email protected]>
>>>
>>> Acked-by: Nicolas Ferre <[email protected]>
>> Applied to the fixes-togreg branch of
>> git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git
>
> Actually, change of plan. Applied to the togreg branch of that
> tree. Whilst technically more correct to have it as you say,
> we are dealing with the size of a struct iio_trig ** vs
> a struct iio_trig * so it isn't actually a bug, just a
> less than ideal bit of code ;)

Absolutely.

> It was so obviously a fix I didn't initially check if it
> was a 'real' bug or not. oops.

Yes, I took this path myself as well ;-)

Bye,


>>>> ---
>>>>
>>>> diff -u -p a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
>>>> --- a/drivers/iio/adc/at91_adc.c
>>>> +++ b/drivers/iio/adc/at91_adc.c
>>>> @@ -279,7 +279,7 @@ static int at91_adc_trigger_init(struct
>>>> int i, ret;
>>>>
>>>> st->trig = devm_kzalloc(&idev->dev,
>>>> - st->trigger_number * sizeof(st->trig),
>>>> + st->trigger_number * sizeof(*st->trig),
>>>> GFP_KERNEL);
>>>>
>>>> if (st->trig == NULL) {
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe devicetree" in
>>>> the body of a message to [email protected]
>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>>
>>>
>>>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>


--
Nicolas Ferre