2014-07-08 22:31:34

by Himangi Saraogi

Subject: [PATCH v3] thunderbolt: Use kcalloc and correct the argument to sizeof

nhi->rx_rings does not have type as struct tb_ring *, as it is a double
pointer. So, the elements of the array should have pointer type, not
structure type. The advantage of kcalloc is, that will prevent integer
overflows which could result from the multiplication of number of
elements and size and it is also a bit nicer to read.

The Coccinelle semantic patch that makes the first change is as follows:

// <smpl>
@disable sizeof_type_expr@
type T;
T **x;
@@

x =
<+...sizeof(
- T
+ *x
)...+>
// </smpl>

Signed-off-by: Himangi Saraogi <[email protected]>
Acked-by: Julia Lawall <[email protected]>
---
v2: Use kcalloc
v3: Add maintainer in To
drivers/thunderbolt/nhi.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c
index 2054fbf..c68fe12 100644
--- a/drivers/thunderbolt/nhi.c
+++ b/drivers/thunderbolt/nhi.c
@@ -569,12 +569,10 @@ static int nhi_probe(struct pci_dev *pdev, const struct pci_device_id *id)
nhi->hop_count);
INIT_WORK(&nhi->interrupt_work, nhi_interrupt_work);

- nhi->tx_rings = devm_kzalloc(&pdev->dev,
- nhi->hop_count * sizeof(struct tb_ring),
- GFP_KERNEL);
- nhi->rx_rings = devm_kzalloc(&pdev->dev,
- nhi->hop_count * sizeof(struct tb_ring),
- GFP_KERNEL);
+ nhi->tx_rings = devm_kcalloc(&pdev->dev, nhi->hop_count,
+ sizeof(*nhi->tx_rings), GFP_KERNEL);
+ nhi->rx_rings = devm_kcalloc(&pdev->dev, nhi->hop_count,
+ sizeof(*nhi->rx_rings), GFP_KERNEL);
if (!nhi->tx_rings || !nhi->rx_rings)
return -ENOMEM;

--
1.9.1

2014-07-10 16:46:15

by Andreas Noever

Subject: Re: [PATCH v3] thunderbolt: Use kcalloc and correct the argument to sizeof

On Wed, Jul 9, 2014 at 12:31 AM, Himangi Saraogi <[email protected]> wrote:
> nhi->rx_rings does not have type as struct tb_ring *, as it is a double
> pointer. So, the elements of the array should have pointer type, not
> structure type. The advantage of kcalloc is, that will prevent integer
> overflows which could result from the multiplication of number of
> elements and size and it is also a bit nicer to read.
>
> The Coccinelle semantic patch that makes the first change is as follows:
>
> // <smpl>
> @disable sizeof_type_expr@
> type T;
> T **x;
> @@
>
> x =
> <+...sizeof(
> - T
> + *x
> )...+>
> // </smpl>
>
> Signed-off-by: Himangi Saraogi <[email protected]>
> Acked-by: Julia Lawall <[email protected]>
> ---
> v2: Use kcalloc
> v3: Add maintainer in To
> drivers/thunderbolt/nhi.c | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c
> index 2054fbf..c68fe12 100644
> --- a/drivers/thunderbolt/nhi.c
> +++ b/drivers/thunderbolt/nhi.c
> @@ -569,12 +569,10 @@ static int nhi_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> nhi->hop_count);
> INIT_WORK(&nhi->interrupt_work, nhi_interrupt_work);
>
> - nhi->tx_rings = devm_kzalloc(&pdev->dev,
> - nhi->hop_count * sizeof(struct tb_ring),
> - GFP_KERNEL);
> - nhi->rx_rings = devm_kzalloc(&pdev->dev,
> - nhi->hop_count * sizeof(struct tb_ring),
> - GFP_KERNEL);
> + nhi->tx_rings = devm_kcalloc(&pdev->dev, nhi->hop_count,
> + sizeof(*nhi->tx_rings), GFP_KERNEL);
> + nhi->rx_rings = devm_kcalloc(&pdev->dev, nhi->hop_count,
> + sizeof(*nhi->rx_rings), GFP_KERNEL);
> if (!nhi->tx_rings || !nhi->rx_rings)
> return -ENOMEM;
>
> --
> 1.9.1
>

Thanks for catching this.
Acked-by: Andreas Noever <[email protected]>

2014-07-11 17:14:50

by Andreas Noever

Subject: Re: [PATCH v3] thunderbolt: Use kcalloc and correct the argument to sizeof

Greg, can you add this to your char-misc tree (instead of the kzalloc
patch, or do you need a rebased patch)?

Thanks,
Andreas

On Thu, Jul 10, 2014 at 6:45 PM, Andreas Noever
<[email protected]> wrote:
> On Wed, Jul 9, 2014 at 12:31 AM, Himangi Saraogi <[email protected]> wrote:
>> nhi->rx_rings does not have type as struct tb_ring *, as it is a double
>> pointer. So, the elements of the array should have pointer type, not
>> structure type. The advantage of kcalloc is, that will prevent integer
>> overflows which could result from the multiplication of number of
>> elements and size and it is also a bit nicer to read.
>>
>> The Coccinelle semantic patch that makes the first change is as follows:
>>
>> // <smpl>
>> @disable sizeof_type_expr@
>> type T;
>> T **x;
>> @@
>>
>> x =
>> <+...sizeof(
>> - T
>> + *x
>> )...+>
>> // </smpl>
>>
>> Signed-off-by: Himangi Saraogi <[email protected]>
>> Acked-by: Julia Lawall <[email protected]>
>> ---
>> v2: Use kcalloc
>> v3: Add maintainer in To
>> drivers/thunderbolt/nhi.c | 10 ++++------
>> 1 file changed, 4 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c
>> index 2054fbf..c68fe12 100644
>> --- a/drivers/thunderbolt/nhi.c
>> +++ b/drivers/thunderbolt/nhi.c
>> @@ -569,12 +569,10 @@ static int nhi_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>> nhi->hop_count);
>> INIT_WORK(&nhi->interrupt_work, nhi_interrupt_work);
>>
>> - nhi->tx_rings = devm_kzalloc(&pdev->dev,
>> - nhi->hop_count * sizeof(struct tb_ring),
>> - GFP_KERNEL);
>> - nhi->rx_rings = devm_kzalloc(&pdev->dev,
>> - nhi->hop_count * sizeof(struct tb_ring),
>> - GFP_KERNEL);
>> + nhi->tx_rings = devm_kcalloc(&pdev->dev, nhi->hop_count,
>> + sizeof(*nhi->tx_rings), GFP_KERNEL);
>> + nhi->rx_rings = devm_kcalloc(&pdev->dev, nhi->hop_count,
>> + sizeof(*nhi->rx_rings), GFP_KERNEL);
>> if (!nhi->tx_rings || !nhi->rx_rings)
>> return -ENOMEM;
>>
>> --
>> 1.9.1
>>
>
> Thanks for catching this.
> Acked-by: Andreas Noever <[email protected]>

2014-07-11 18:08:56

by Greg KH

Subject: Re: [PATCH v3] thunderbolt: Use kcalloc and correct the argument to sizeof

On Fri, Jul 11, 2014 at 07:14:23PM +0200, Andreas Noever wrote:
> Greg, can you add this to your char-misc tree (instead of the kzalloc
> patch, or do you need a rebased patch)?

I need a patch on top of my char-misc tree as the kzalloc version is
already applied there.

thanks,

greg k-h