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

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

[Hauke Mehrtens]

On 09/19/2013 08:38 PM, thomas@m3y3r.de 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 <thomas@m3y3r.de>
Acked-by: Hauke Mehrtens <hauke@hauke-m.de>

> 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,
> 

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

[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 <thomas@m3y3r.de>
---

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) {

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

[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 <thomas@m3y3r.de>
---

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;
 	}

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

[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 <thomas@m3y3r.de>
---

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 */

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

[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 <thomas@m3y3r.de>
---

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 */

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

[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 <thomas@m3y3r.de>
---

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;
 

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

[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 <thomas@m3y3r.de>
---

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++) {

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

[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 <thomas@m3y3r.de>
---

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;

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

[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 <thomas@m3y3r.de>
---

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) {

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

[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 <thomas@m3y3r.de>
---

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,

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

[Thomas Meyer]

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.

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

[Joe Perches]

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.

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

[Nicolas Ferre]

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 <thomas@m3y3r.de>

Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>

> ---
>
> 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


-- 
Nicolas Ferre

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

[Dan Carpenter]

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 <thomas@m3y3r.de>
> ---
> 
> 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

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

[Dan Carpenter]

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 <thomas@m3y3r.de>
> ---
> 
> 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

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

[Jonathan Cameron]

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 <thomas@m3y3r.de>
> 
> Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
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 majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
> 
> 

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

[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 <thomas@m3y3r.de>
>>
>> Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> 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 majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>
>>
>>

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

[Nicolas Ferre]

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 <thomas@m3y3r.de>
>>>
>>> Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
>> 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 majordomo@vger.kernel.org
>>>> 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


-- 
Nicolas Ferre