* [PATCH] powerpc/powernv/npu: Allocate enough memory in pnv_try_setup_npu_table_group()
@ 2019-01-09 10:23 ` Dan Carpenter
0 siblings, 0 replies; 16+ messages in thread
From: Dan Carpenter @ 2019-01-09 10:23 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Alexey Kardashevskiy
Cc: Alexey Kardashevskiy, Mark Hairgrove, kernel-janitors,
Paul Mackerras, Alistair Popple, linuxppc-dev
There is a typo so we accidentally allocate enough memory for a pointer
when we wanted to allocate enough for a struct.
Fixes: 0bd971676e68 ("powerpc/powernv/npu: Add compound IOMMU groups")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
arch/powerpc/platforms/powernv/npu-dma.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/powernv/npu-dma.c b/arch/powerpc/platforms/powernv/npu-dma.c
index d7f742ed48ba..3f58c7dbd581 100644
--- a/arch/powerpc/platforms/powernv/npu-dma.c
+++ b/arch/powerpc/platforms/powernv/npu-dma.c
@@ -564,7 +564,7 @@ struct iommu_table_group *pnv_try_setup_npu_table_group(struct pnv_ioda_pe *pe)
}
} else {
/* Create a group for 1 GPU and attached NPUs for POWER8 */
- pe->npucomp = kzalloc(sizeof(pe->npucomp), GFP_KERNEL);
+ pe->npucomp = kzalloc(sizeof(*pe->npucomp), GFP_KERNEL);
table_group = &pe->npucomp->table_group;
table_group->ops = &pnv_npu_peers_ops;
iommu_register_group(table_group, hose->global_number,
--
2.17.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH] powerpc/powernv/npu: Allocate enough memory in pnv_try_setup_npu_table_group()
2019-01-09 10:23 ` Dan Carpenter
@ 2019-01-09 11:54 ` Michael Ellerman
-1 siblings, 0 replies; 16+ messages in thread
From: Michael Ellerman @ 2019-01-09 11:54 UTC (permalink / raw)
To: Dan Carpenter, Benjamin Herrenschmidt, Alexey Kardashevskiy
Cc: Alistair Popple, Mark Hairgrove, kernel-janitors, Paul Mackerras,
linuxppc-dev
Dan Carpenter <dan.carpenter@oracle.com> writes:
> There is a typo so we accidentally allocate enough memory for a pointer
> when we wanted to allocate enough for a struct.
>
> Fixes: 0bd971676e68 ("powerpc/powernv/npu: Add compound IOMMU groups")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> arch/powerpc/platforms/powernv/npu-dma.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Thanks, I've applied this to my fixes-test tree.
Alexey can you send me an ack?
cheers
> diff --git a/arch/powerpc/platforms/powernv/npu-dma.c b/arch/powerpc/platforms/powernv/npu-dma.c
> index d7f742ed48ba..3f58c7dbd581 100644
> --- a/arch/powerpc/platforms/powernv/npu-dma.c
> +++ b/arch/powerpc/platforms/powernv/npu-dma.c
> @@ -564,7 +564,7 @@ struct iommu_table_group *pnv_try_setup_npu_table_group(struct pnv_ioda_pe *pe)
> }
> } else {
> /* Create a group for 1 GPU and attached NPUs for POWER8 */
> - pe->npucomp = kzalloc(sizeof(pe->npucomp), GFP_KERNEL);
> + pe->npucomp = kzalloc(sizeof(*pe->npucomp), GFP_KERNEL);
> table_group = &pe->npucomp->table_group;
> table_group->ops = &pnv_npu_peers_ops;
> iommu_register_group(table_group, hose->global_number,
> --
> 2.17.1
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH] powerpc/powernv/npu: Allocate enough memory in pnv_try_setup_npu_table_group()
@ 2019-01-09 11:54 ` Michael Ellerman
0 siblings, 0 replies; 16+ messages in thread
From: Michael Ellerman @ 2019-01-09 11:54 UTC (permalink / raw)
To: Dan Carpenter, Benjamin Herrenschmidt, Alexey Kardashevskiy
Cc: Alexey Kardashevskiy, Alistair Popple, Mark Hairgrove,
kernel-janitors, Paul Mackerras, linuxppc-dev
Dan Carpenter <dan.carpenter@oracle.com> writes:
> There is a typo so we accidentally allocate enough memory for a pointer
> when we wanted to allocate enough for a struct.
>
> Fixes: 0bd971676e68 ("powerpc/powernv/npu: Add compound IOMMU groups")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> arch/powerpc/platforms/powernv/npu-dma.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Thanks, I've applied this to my fixes-test tree.
Alexey can you send me an ack?
cheers
> diff --git a/arch/powerpc/platforms/powernv/npu-dma.c b/arch/powerpc/platforms/powernv/npu-dma.c
> index d7f742ed48ba..3f58c7dbd581 100644
> --- a/arch/powerpc/platforms/powernv/npu-dma.c
> +++ b/arch/powerpc/platforms/powernv/npu-dma.c
> @@ -564,7 +564,7 @@ struct iommu_table_group *pnv_try_setup_npu_table_group(struct pnv_ioda_pe *pe)
> }
> } else {
> /* Create a group for 1 GPU and attached NPUs for POWER8 */
> - pe->npucomp = kzalloc(sizeof(pe->npucomp), GFP_KERNEL);
> + pe->npucomp = kzalloc(sizeof(*pe->npucomp), GFP_KERNEL);
> table_group = &pe->npucomp->table_group;
> table_group->ops = &pnv_npu_peers_ops;
> iommu_register_group(table_group, hose->global_number,
> --
> 2.17.1
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH] powerpc/powernv/npu: Allocate enough memory in pnv_try_setup_npu_table_group()
2019-01-09 11:54 ` Michael Ellerman
@ 2019-01-11 2:12 ` Alexey Kardashevskiy
-1 siblings, 0 replies; 16+ messages in thread
From: Alexey Kardashevskiy @ 2019-01-11 2:12 UTC (permalink / raw)
To: Michael Ellerman, Dan Carpenter, Benjamin Herrenschmidt
Cc: Alistair Popple, Mark Hairgrove, kernel-janitors, Paul Mackerras,
linuxppc-dev
On 09/01/2019 22:54, Michael Ellerman wrote:
> Dan Carpenter <dan.carpenter@oracle.com> writes:
>> There is a typo so we accidentally allocate enough memory for a pointer
>> when we wanted to allocate enough for a struct.
>>
>> Fixes: 0bd971676e68 ("powerpc/powernv/npu: Add compound IOMMU groups")
>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>> ---
>> arch/powerpc/platforms/powernv/npu-dma.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> Thanks, I've applied this to my fixes-test tree.
>
> Alexey can you send me an ack?
Ouch.
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>
> cheers
>
>> diff --git a/arch/powerpc/platforms/powernv/npu-dma.c b/arch/powerpc/platforms/powernv/npu-dma.c
>> index d7f742ed48ba..3f58c7dbd581 100644
>> --- a/arch/powerpc/platforms/powernv/npu-dma.c
>> +++ b/arch/powerpc/platforms/powernv/npu-dma.c
>> @@ -564,7 +564,7 @@ struct iommu_table_group *pnv_try_setup_npu_table_group(struct pnv_ioda_pe *pe)
>> }
>> } else {
>> /* Create a group for 1 GPU and attached NPUs for POWER8 */
>> - pe->npucomp = kzalloc(sizeof(pe->npucomp), GFP_KERNEL);
>> + pe->npucomp = kzalloc(sizeof(*pe->npucomp), GFP_KERNEL);
>> table_group = &pe->npucomp->table_group;
>> table_group->ops = &pnv_npu_peers_ops;
>> iommu_register_group(table_group, hose->global_number,
>> --
>> 2.17.1
--
Alexey
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH] powerpc/powernv/npu: Allocate enough memory in pnv_try_setup_npu_table_group()
@ 2019-01-11 2:12 ` Alexey Kardashevskiy
0 siblings, 0 replies; 16+ messages in thread
From: Alexey Kardashevskiy @ 2019-01-11 2:12 UTC (permalink / raw)
To: Michael Ellerman, Dan Carpenter, Benjamin Herrenschmidt
Cc: Alistair Popple, Mark Hairgrove, kernel-janitors, Paul Mackerras,
linuxppc-dev
On 09/01/2019 22:54, Michael Ellerman wrote:
> Dan Carpenter <dan.carpenter@oracle.com> writes:
>> There is a typo so we accidentally allocate enough memory for a pointer
>> when we wanted to allocate enough for a struct.
>>
>> Fixes: 0bd971676e68 ("powerpc/powernv/npu: Add compound IOMMU groups")
>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>> ---
>> arch/powerpc/platforms/powernv/npu-dma.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> Thanks, I've applied this to my fixes-test tree.
>
> Alexey can you send me an ack?
Ouch.
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>
> cheers
>
>> diff --git a/arch/powerpc/platforms/powernv/npu-dma.c b/arch/powerpc/platforms/powernv/npu-dma.c
>> index d7f742ed48ba..3f58c7dbd581 100644
>> --- a/arch/powerpc/platforms/powernv/npu-dma.c
>> +++ b/arch/powerpc/platforms/powernv/npu-dma.c
>> @@ -564,7 +564,7 @@ struct iommu_table_group *pnv_try_setup_npu_table_group(struct pnv_ioda_pe *pe)
>> }
>> } else {
>> /* Create a group for 1 GPU and attached NPUs for POWER8 */
>> - pe->npucomp = kzalloc(sizeof(pe->npucomp), GFP_KERNEL);
>> + pe->npucomp = kzalloc(sizeof(*pe->npucomp), GFP_KERNEL);
>> table_group = &pe->npucomp->table_group;
>> table_group->ops = &pnv_npu_peers_ops;
>> iommu_register_group(table_group, hose->global_number,
>> --
>> 2.17.1
--
Alexey
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] powerpc/powernv/npu: Allocate enough memory in pnv_try_setup_npu_table_group()
2019-01-09 10:23 ` Dan Carpenter
@ 2019-01-12 0:30 ` Balbir Singh
-1 siblings, 0 replies; 16+ messages in thread
From: Balbir Singh @ 2019-01-12 0:30 UTC (permalink / raw)
To: Dan Carpenter
Cc: Alexey Kardashevskiy, Mark Hairgrove, kernel-janitors,
Paul Mackerras, Alistair Popple, linuxppc-dev
On Wed, Jan 09, 2019 at 01:23:29PM +0300, Dan Carpenter wrote:
> There is a typo so we accidentally allocate enough memory for a pointer
> when we wanted to allocate enough for a struct.
>
> Fixes: 0bd971676e68 ("powerpc/powernv/npu: Add compound IOMMU groups")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> arch/powerpc/platforms/powernv/npu-dma.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/platforms/powernv/npu-dma.c b/arch/powerpc/platforms/powernv/npu-dma.c
> index d7f742ed48ba..3f58c7dbd581 100644
> --- a/arch/powerpc/platforms/powernv/npu-dma.c
> +++ b/arch/powerpc/platforms/powernv/npu-dma.c
> @@ -564,7 +564,7 @@ struct iommu_table_group *pnv_try_setup_npu_table_group(struct pnv_ioda_pe *pe)
> }
> } else {
> /* Create a group for 1 GPU and attached NPUs for POWER8 */
> - pe->npucomp = kzalloc(sizeof(pe->npucomp), GFP_KERNEL);
> + pe->npucomp = kzalloc(sizeof(*pe->npucomp), GFP_KERNEL);
To avoid these in the future, I wonder if instead of sizeof(pe->npucomp), we insist on
sizeof structure
pe->npucomp = kzalloc(sizeof(struct npucomp), GFP_KERNEL);
Acked-by: Balbir Singh <bsingharora@gmail.com>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH] powerpc/powernv/npu: Allocate enough memory in pnv_try_setup_npu_table_group()
@ 2019-01-12 0:30 ` Balbir Singh
0 siblings, 0 replies; 16+ messages in thread
From: Balbir Singh @ 2019-01-12 0:30 UTC (permalink / raw)
To: Dan Carpenter
Cc: Alexey Kardashevskiy, Mark Hairgrove, kernel-janitors,
Paul Mackerras, Alistair Popple, linuxppc-dev
On Wed, Jan 09, 2019 at 01:23:29PM +0300, Dan Carpenter wrote:
> There is a typo so we accidentally allocate enough memory for a pointer
> when we wanted to allocate enough for a struct.
>
> Fixes: 0bd971676e68 ("powerpc/powernv/npu: Add compound IOMMU groups")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> arch/powerpc/platforms/powernv/npu-dma.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/platforms/powernv/npu-dma.c b/arch/powerpc/platforms/powernv/npu-dma.c
> index d7f742ed48ba..3f58c7dbd581 100644
> --- a/arch/powerpc/platforms/powernv/npu-dma.c
> +++ b/arch/powerpc/platforms/powernv/npu-dma.c
> @@ -564,7 +564,7 @@ struct iommu_table_group *pnv_try_setup_npu_table_group(struct pnv_ioda_pe *pe)
> }
> } else {
> /* Create a group for 1 GPU and attached NPUs for POWER8 */
> - pe->npucomp = kzalloc(sizeof(pe->npucomp), GFP_KERNEL);
> + pe->npucomp = kzalloc(sizeof(*pe->npucomp), GFP_KERNEL);
To avoid these in the future, I wonder if instead of sizeof(pe->npucomp), we insist on
sizeof structure
pe->npucomp = kzalloc(sizeof(struct npucomp), GFP_KERNEL);
Acked-by: Balbir Singh <bsingharora@gmail.com>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH] powerpc/powernv/npu: Allocate enough memory in pnv_try_setup_npu_table_group()
2019-01-12 0:30 ` Balbir Singh
@ 2019-01-12 5:44 ` Dan Carpenter
-1 siblings, 0 replies; 16+ messages in thread
From: Dan Carpenter @ 2019-01-12 5:44 UTC (permalink / raw)
To: Balbir Singh
Cc: Alexey Kardashevskiy, Mark Hairgrove, kernel-janitors,
Paul Mackerras, Alistair Popple, linuxppc-dev
On Sat, Jan 12, 2019 at 11:30:35AM +1100, Balbir Singh wrote:
> On Wed, Jan 09, 2019 at 01:23:29PM +0300, Dan Carpenter wrote:
> > There is a typo so we accidentally allocate enough memory for a pointer
> > when we wanted to allocate enough for a struct.
> >
> > Fixes: 0bd971676e68 ("powerpc/powernv/npu: Add compound IOMMU groups")
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > ---
> > arch/powerpc/platforms/powernv/npu-dma.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/powerpc/platforms/powernv/npu-dma.c b/arch/powerpc/platforms/powernv/npu-dma.c
> > index d7f742ed48ba..3f58c7dbd581 100644
> > --- a/arch/powerpc/platforms/powernv/npu-dma.c
> > +++ b/arch/powerpc/platforms/powernv/npu-dma.c
> > @@ -564,7 +564,7 @@ struct iommu_table_group *pnv_try_setup_npu_table_group(struct pnv_ioda_pe *pe)
> > }
> > } else {
> > /* Create a group for 1 GPU and attached NPUs for POWER8 */
> > - pe->npucomp = kzalloc(sizeof(pe->npucomp), GFP_KERNEL);
> > + pe->npucomp = kzalloc(sizeof(*pe->npucomp), GFP_KERNEL);
>
> To avoid these in the future, I wonder if instead of sizeof(pe->npucomp), we insist on
> sizeof structure
>
> pe->npucomp = kzalloc(sizeof(struct npucomp), GFP_KERNEL);
>
The latest kernel fashion is sizeof(*ptr). It can go wrong either way.
I don't have strong feelings about it. These sorts of bugs don't last
long because they're caught in testing or with static analysis.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH] powerpc/powernv/npu: Allocate enough memory in pnv_try_setup_npu_table_group()
@ 2019-01-12 5:44 ` Dan Carpenter
0 siblings, 0 replies; 16+ messages in thread
From: Dan Carpenter @ 2019-01-12 5:44 UTC (permalink / raw)
To: Balbir Singh
Cc: Alexey Kardashevskiy, Mark Hairgrove, kernel-janitors,
Paul Mackerras, Alistair Popple, linuxppc-dev
On Sat, Jan 12, 2019 at 11:30:35AM +1100, Balbir Singh wrote:
> On Wed, Jan 09, 2019 at 01:23:29PM +0300, Dan Carpenter wrote:
> > There is a typo so we accidentally allocate enough memory for a pointer
> > when we wanted to allocate enough for a struct.
> >
> > Fixes: 0bd971676e68 ("powerpc/powernv/npu: Add compound IOMMU groups")
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > ---
> > arch/powerpc/platforms/powernv/npu-dma.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/powerpc/platforms/powernv/npu-dma.c b/arch/powerpc/platforms/powernv/npu-dma.c
> > index d7f742ed48ba..3f58c7dbd581 100644
> > --- a/arch/powerpc/platforms/powernv/npu-dma.c
> > +++ b/arch/powerpc/platforms/powernv/npu-dma.c
> > @@ -564,7 +564,7 @@ struct iommu_table_group *pnv_try_setup_npu_table_group(struct pnv_ioda_pe *pe)
> > }
> > } else {
> > /* Create a group for 1 GPU and attached NPUs for POWER8 */
> > - pe->npucomp = kzalloc(sizeof(pe->npucomp), GFP_KERNEL);
> > + pe->npucomp = kzalloc(sizeof(*pe->npucomp), GFP_KERNEL);
>
> To avoid these in the future, I wonder if instead of sizeof(pe->npucomp), we insist on
> sizeof structure
>
> pe->npucomp = kzalloc(sizeof(struct npucomp), GFP_KERNEL);
>
The latest kernel fashion is sizeof(*ptr). It can go wrong either way.
I don't have strong feelings about it. These sorts of bugs don't last
long because they're caught in testing or with static analysis.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH] powerpc/powernv/npu: Allocate enough memory in pnv_try_setup_npu_table_group()
2019-01-12 5:44 ` Dan Carpenter
@ 2019-01-12 7:34 ` Segher Boessenkool
-1 siblings, 0 replies; 16+ messages in thread
From: Segher Boessenkool @ 2019-01-12 7:34 UTC (permalink / raw)
To: Dan Carpenter
Cc: Alexey Kardashevskiy, Alistair Popple, Mark Hairgrove,
kernel-janitors, Paul Mackerras, linuxppc-dev
On Sat, Jan 12, 2019 at 08:44:26AM +0300, Dan Carpenter wrote:
> On Sat, Jan 12, 2019 at 11:30:35AM +1100, Balbir Singh wrote:
> > On Wed, Jan 09, 2019 at 01:23:29PM +0300, Dan Carpenter wrote:
> > > There is a typo so we accidentally allocate enough memory for a pointer
> > > when we wanted to allocate enough for a struct.
> > >
> > > Fixes: 0bd971676e68 ("powerpc/powernv/npu: Add compound IOMMU groups")
> > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > > ---
> > > arch/powerpc/platforms/powernv/npu-dma.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/arch/powerpc/platforms/powernv/npu-dma.c b/arch/powerpc/platforms/powernv/npu-dma.c
> > > index d7f742ed48ba..3f58c7dbd581 100644
> > > --- a/arch/powerpc/platforms/powernv/npu-dma.c
> > > +++ b/arch/powerpc/platforms/powernv/npu-dma.c
> > > @@ -564,7 +564,7 @@ struct iommu_table_group *pnv_try_setup_npu_table_group(struct pnv_ioda_pe *pe)
> > > }
> > > } else {
> > > /* Create a group for 1 GPU and attached NPUs for POWER8 */
> > > - pe->npucomp = kzalloc(sizeof(pe->npucomp), GFP_KERNEL);
> > > + pe->npucomp = kzalloc(sizeof(*pe->npucomp), GFP_KERNEL);
> >
> > To avoid these in the future, I wonder if instead of sizeof(pe->npucomp), we insist on
> > sizeof structure
> >
> > pe->npucomp = kzalloc(sizeof(struct npucomp), GFP_KERNEL);
> >
>
> The latest kernel fashion is sizeof(*ptr). It can go wrong either way.
> I don't have strong feelings about it. These sorts of bugs don't last
> long because they're caught in testing or with static analysis.
And it is easy to see someone forgot the * in "sizeof *ptr", and with
experience it will just automatically look wrong if it is forgotten; but
it isn't obvious at all if the wrong struct is used, which cannot happen
with the *ptr form, but happens frequently with the "sizeof(struct x)"
form.
Segher
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH] powerpc/powernv/npu: Allocate enough memory in pnv_try_setup_npu_table_group()
@ 2019-01-12 7:34 ` Segher Boessenkool
0 siblings, 0 replies; 16+ messages in thread
From: Segher Boessenkool @ 2019-01-12 7:34 UTC (permalink / raw)
To: Dan Carpenter
Cc: Alexey Kardashevskiy, Alistair Popple, Mark Hairgrove,
kernel-janitors, Paul Mackerras, linuxppc-dev
On Sat, Jan 12, 2019 at 08:44:26AM +0300, Dan Carpenter wrote:
> On Sat, Jan 12, 2019 at 11:30:35AM +1100, Balbir Singh wrote:
> > On Wed, Jan 09, 2019 at 01:23:29PM +0300, Dan Carpenter wrote:
> > > There is a typo so we accidentally allocate enough memory for a pointer
> > > when we wanted to allocate enough for a struct.
> > >
> > > Fixes: 0bd971676e68 ("powerpc/powernv/npu: Add compound IOMMU groups")
> > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > > ---
> > > arch/powerpc/platforms/powernv/npu-dma.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/arch/powerpc/platforms/powernv/npu-dma.c b/arch/powerpc/platforms/powernv/npu-dma.c
> > > index d7f742ed48ba..3f58c7dbd581 100644
> > > --- a/arch/powerpc/platforms/powernv/npu-dma.c
> > > +++ b/arch/powerpc/platforms/powernv/npu-dma.c
> > > @@ -564,7 +564,7 @@ struct iommu_table_group *pnv_try_setup_npu_table_group(struct pnv_ioda_pe *pe)
> > > }
> > > } else {
> > > /* Create a group for 1 GPU and attached NPUs for POWER8 */
> > > - pe->npucomp = kzalloc(sizeof(pe->npucomp), GFP_KERNEL);
> > > + pe->npucomp = kzalloc(sizeof(*pe->npucomp), GFP_KERNEL);
> >
> > To avoid these in the future, I wonder if instead of sizeof(pe->npucomp), we insist on
> > sizeof structure
> >
> > pe->npucomp = kzalloc(sizeof(struct npucomp), GFP_KERNEL);
> >
>
> The latest kernel fashion is sizeof(*ptr). It can go wrong either way.
> I don't have strong feelings about it. These sorts of bugs don't last
> long because they're caught in testing or with static analysis.
And it is easy to see someone forgot the * in "sizeof *ptr", and with
experience it will just automatically look wrong if it is forgotten; but
it isn't obvious at all if the wrong struct is used, which cannot happen
with the *ptr form, but happens frequently with the "sizeof(struct x)"
form.
Segher
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH] powerpc/powernv/npu: Allocate enough memory in pnv_try_setup_npu_table_group()
2019-01-12 7:34 ` Segher Boessenkool
@ 2019-01-12 10:51 ` Dan Carpenter
-1 siblings, 0 replies; 16+ messages in thread
From: Dan Carpenter @ 2019-01-12 10:51 UTC (permalink / raw)
To: Segher Boessenkool
Cc: Alexey Kardashevskiy, Alistair Popple, Mark Hairgrove,
kernel-janitors, Paul Mackerras, linuxppc-dev
On Sat, Jan 12, 2019 at 01:34:42AM -0600, Segher Boessenkool wrote:
> On Sat, Jan 12, 2019 at 08:44:26AM +0300, Dan Carpenter wrote:
> > On Sat, Jan 12, 2019 at 11:30:35AM +1100, Balbir Singh wrote:
> > > On Wed, Jan 09, 2019 at 01:23:29PM +0300, Dan Carpenter wrote:
> > > > There is a typo so we accidentally allocate enough memory for a pointer
> > > > when we wanted to allocate enough for a struct.
> > > >
> > > > Fixes: 0bd971676e68 ("powerpc/powernv/npu: Add compound IOMMU groups")
> > > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > > > ---
> > > > arch/powerpc/platforms/powernv/npu-dma.c | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/arch/powerpc/platforms/powernv/npu-dma.c b/arch/powerpc/platforms/powernv/npu-dma.c
> > > > index d7f742ed48ba..3f58c7dbd581 100644
> > > > --- a/arch/powerpc/platforms/powernv/npu-dma.c
> > > > +++ b/arch/powerpc/platforms/powernv/npu-dma.c
> > > > @@ -564,7 +564,7 @@ struct iommu_table_group *pnv_try_setup_npu_table_group(struct pnv_ioda_pe *pe)
> > > > }
> > > > } else {
> > > > /* Create a group for 1 GPU and attached NPUs for POWER8 */
> > > > - pe->npucomp = kzalloc(sizeof(pe->npucomp), GFP_KERNEL);
> > > > + pe->npucomp = kzalloc(sizeof(*pe->npucomp), GFP_KERNEL);
> > >
> > > To avoid these in the future, I wonder if instead of sizeof(pe->npucomp), we insist on
> > > sizeof structure
> > >
> > > pe->npucomp = kzalloc(sizeof(struct npucomp), GFP_KERNEL);
> > >
> >
> > The latest kernel fashion is sizeof(*ptr). It can go wrong either way.
> > I don't have strong feelings about it. These sorts of bugs don't last
> > long because they're caught in testing or with static analysis.
>
> And it is easy to see someone forgot the * in "sizeof *ptr", and with
> experience it will just automatically look wrong if it is forgotten; but
> it isn't obvious at all if the wrong struct is used, which cannot happen
> with the *ptr form, but happens frequently with the "sizeof(struct x)"
> form.
It doesn't happen very frequently. I look for this with Smatch. The
results I see are when the first few members of a struct are a header
and the actual size can vary.
hdr = alloc(sizeof(struct larger_struct));
regards,
dan carpenter
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH] powerpc/powernv/npu: Allocate enough memory in pnv_try_setup_npu_table_group()
@ 2019-01-12 10:51 ` Dan Carpenter
0 siblings, 0 replies; 16+ messages in thread
From: Dan Carpenter @ 2019-01-12 10:51 UTC (permalink / raw)
To: Segher Boessenkool
Cc: Alexey Kardashevskiy, Alistair Popple, Mark Hairgrove,
kernel-janitors, Paul Mackerras, linuxppc-dev
On Sat, Jan 12, 2019 at 01:34:42AM -0600, Segher Boessenkool wrote:
> On Sat, Jan 12, 2019 at 08:44:26AM +0300, Dan Carpenter wrote:
> > On Sat, Jan 12, 2019 at 11:30:35AM +1100, Balbir Singh wrote:
> > > On Wed, Jan 09, 2019 at 01:23:29PM +0300, Dan Carpenter wrote:
> > > > There is a typo so we accidentally allocate enough memory for a pointer
> > > > when we wanted to allocate enough for a struct.
> > > >
> > > > Fixes: 0bd971676e68 ("powerpc/powernv/npu: Add compound IOMMU groups")
> > > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > > > ---
> > > > arch/powerpc/platforms/powernv/npu-dma.c | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/arch/powerpc/platforms/powernv/npu-dma.c b/arch/powerpc/platforms/powernv/npu-dma.c
> > > > index d7f742ed48ba..3f58c7dbd581 100644
> > > > --- a/arch/powerpc/platforms/powernv/npu-dma.c
> > > > +++ b/arch/powerpc/platforms/powernv/npu-dma.c
> > > > @@ -564,7 +564,7 @@ struct iommu_table_group *pnv_try_setup_npu_table_group(struct pnv_ioda_pe *pe)
> > > > }
> > > > } else {
> > > > /* Create a group for 1 GPU and attached NPUs for POWER8 */
> > > > - pe->npucomp = kzalloc(sizeof(pe->npucomp), GFP_KERNEL);
> > > > + pe->npucomp = kzalloc(sizeof(*pe->npucomp), GFP_KERNEL);
> > >
> > > To avoid these in the future, I wonder if instead of sizeof(pe->npucomp), we insist on
> > > sizeof structure
> > >
> > > pe->npucomp = kzalloc(sizeof(struct npucomp), GFP_KERNEL);
> > >
> >
> > The latest kernel fashion is sizeof(*ptr). It can go wrong either way.
> > I don't have strong feelings about it. These sorts of bugs don't last
> > long because they're caught in testing or with static analysis.
>
> And it is easy to see someone forgot the * in "sizeof *ptr", and with
> experience it will just automatically look wrong if it is forgotten; but
> it isn't obvious at all if the wrong struct is used, which cannot happen
> with the *ptr form, but happens frequently with the "sizeof(struct x)"
> form.
It doesn't happen very frequently. I look for this with Smatch. The
results I see are when the first few members of a struct are a header
and the actual size can vary.
hdr = alloc(sizeof(struct larger_struct));
regards,
dan carpenter
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: powerpc/powernv/npu: Allocate enough memory in pnv_try_setup_npu_table_group()
2019-01-09 10:23 ` Dan Carpenter
@ 2019-01-14 10:12 ` Michael Ellerman
-1 siblings, 0 replies; 16+ messages in thread
From: Michael Ellerman @ 2019-01-14 10:12 UTC (permalink / raw)
To: Dan Carpenter, Benjamin Herrenschmidt, Alexey Kardashevskiy
Cc: Alistair Popple, Mark Hairgrove, kernel-janitors, Paul Mackerras,
linuxppc-dev
On Wed, 2019-01-09 at 10:23:29 UTC, Dan Carpenter wrote:
> There is a typo so we accidentally allocate enough memory for a pointer
> when we wanted to allocate enough for a struct.
>
> Fixes: 0bd971676e68 ("powerpc/powernv/npu: Add compound IOMMU groups")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> Acked-by: Balbir Singh <bsingharora@gmail.com>
Applied to powerpc fixes, thanks.
https://git.kernel.org/powerpc/c/d7b6cc199b2dea602b4a2a681cf6d322
cheers
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: powerpc/powernv/npu: Allocate enough memory in pnv_try_setup_npu_table_group()
@ 2019-01-14 10:12 ` Michael Ellerman
0 siblings, 0 replies; 16+ messages in thread
From: Michael Ellerman @ 2019-01-14 10:12 UTC (permalink / raw)
To: Dan Carpenter, Benjamin Herrenschmidt, Alexey Kardashevskiy
Cc: Alexey Kardashevskiy, Alistair Popple, Mark Hairgrove,
kernel-janitors, Paul Mackerras, linuxppc-dev
On Wed, 2019-01-09 at 10:23:29 UTC, Dan Carpenter wrote:
> There is a typo so we accidentally allocate enough memory for a pointer
> when we wanted to allocate enough for a struct.
>
> Fixes: 0bd971676e68 ("powerpc/powernv/npu: Add compound IOMMU groups")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> Acked-by: Balbir Singh <bsingharora@gmail.com>
Applied to powerpc fixes, thanks.
https://git.kernel.org/powerpc/c/d7b6cc199b2dea602b4a2a681cf6d322
cheers
^ permalink raw reply [flat|nested] 16+ messages in thread