[PATCH 0/2 net-next] cxgb3/l2t: Fix undefined behaviour and use struct_size() helper

[PATCH 0/2 net-next] cxgb3/l2t: Fix undefined behaviour and use struct_size() helper

[Gustavo A. R. Silva]

Hi all,

This patchset aims to fix an undefined behaviour when using a zero-sized
array and, add the use of the struct_size() helper in kvzalloc().

You might consider the first patch in this series for stable.

More details in the commit logs.

Thanks

Gustavo A. R. Silva (2):
  cxgb3/l2t: Fix undefined behaviour
  cxgb3/l2t: Use struct_size() in kvzalloc()

 drivers/net/ethernet/chelsio/cxgb3/l2t.c | 4 ++--
 drivers/net/ethernet/chelsio/cxgb3/l2t.h | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

-- 
2.21.0

[PATCH 1/2 net-next] cxgb3/l2t: Fix undefined behaviour

[Gustavo A. R. Silva]

The use of zero-sized array causes undefined behaviour when it is not
the last member in a structure. As it happens to be in this case.

Also, the current code makes use of a language extension to the C90
standard, but the preferred mechanism to declare variable-length
types such as this one is a flexible array member, introduced in
C99:

struct foo {
        int stuff;
        struct boo array[];
};

By making use of the mechanism above, we will get a compiler warning
in case the flexible array does not occur last. Which is beneficial
to cultivate a high-quality code.

Fixes: e48f129c2f20 ("[SCSI] cxgb3i: convert cdev->l2opt to use rcu to prevent NULL dereference")
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
 drivers/net/ethernet/chelsio/cxgb3/l2t.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/chelsio/cxgb3/l2t.h b/drivers/net/ethernet/chelsio/cxgb3/l2t.h
index c2fd323c4078..ea75f275023f 100644
--- a/drivers/net/ethernet/chelsio/cxgb3/l2t.h
+++ b/drivers/net/ethernet/chelsio/cxgb3/l2t.h
@@ -75,8 +75,8 @@ struct l2t_data {
 	struct l2t_entry *rover;	/* starting point for next allocation */
 	atomic_t nfree;		/* number of free entries */
 	rwlock_t lock;
-	struct l2t_entry l2tab[0];
 	struct rcu_head rcu_head;	/* to handle rcu cleanup */
+	struct l2t_entry l2tab[];
 };
 
 typedef void (*arp_failure_handler_func)(struct t3cdev * dev,
-- 
2.21.0

[PATCH 2/2 net-next] cxgb3/l2t: Use struct_size() in kvzalloc()

[Gustavo A. R. Silva]

One of the more common cases of allocation size calculations is finding
the size of a structure that has a zero-sized array at the end, along
with memory for some number of elements for that array. For example:

struct foo {
    int stuff;
    struct boo entry[];
};

size = sizeof(struct foo) + count * sizeof(struct boo);
instance = kvzalloc(size, GFP_KERNEL);

Instead of leaving these open-coded and prone to type mistakes, we can
now use the new struct_size() helper:

instance = kvzalloc(struct_size(instance, entry, count), GFP_KERNEL);

Notice that, in this case, variable size is not necessary, hence
it is removed.

This code was detected with the help of Coccinelle.

Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
 drivers/net/ethernet/chelsio/cxgb3/l2t.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/chelsio/cxgb3/l2t.c b/drivers/net/ethernet/chelsio/cxgb3/l2t.c
index 0e9182d3f02c..b3e4118a15e7 100644
--- a/drivers/net/ethernet/chelsio/cxgb3/l2t.c
+++ b/drivers/net/ethernet/chelsio/cxgb3/l2t.c
@@ -443,9 +443,9 @@ void t3_l2t_update(struct t3cdev *dev, struct neighbour *neigh)
 struct l2t_data *t3_init_l2t(unsigned int l2t_capacity)
 {
 	struct l2t_data *d;
-	int i, size = sizeof(*d) + l2t_capacity * sizeof(struct l2t_entry);
+	int i;
 
-	d = kvzalloc(size, GFP_KERNEL);
+	d = kvzalloc(struct_size(d, l2tab, l2t_capacity), GFP_KERNEL);
 	if (!d)
 		return NULL;
 
-- 
2.21.0

Re: [PATCH 0/2 net-next] cxgb3/l2t: Fix undefined behaviour and use struct_size() helper

[David Miller]

From: "Gustavo A. R. Silva" <gustavo@embeddedor.com>
Date: Fri, 29 Mar 2019 10:26:16 -0500

> Hi all,
> 
> This patchset aims to fix an undefined behaviour when using a zero-sized
> array and, add the use of the struct_size() helper in kvzalloc().
> 
> You might consider the first patch in this series for stable.
> 
> More details in the commit logs.

Series applied, honestly I don't think this is -stable material.

And if it was you should have targetted 'net' instead of 'net-next'.

Re: [PATCH 0/2 net-next] cxgb3/l2t: Fix undefined behaviour and use struct_size() helper

[Gustavo A. R. Silva]

On 4/1/19 5:02 PM, David Miller wrote:
> From: "Gustavo A. R. Silva" <gustavo@embeddedor.com>
> Date: Fri, 29 Mar 2019 10:26:16 -0500
> 
>> Hi all,
>>
>> This patchset aims to fix an undefined behaviour when using a zero-sized
>> array and, add the use of the struct_size() helper in kvzalloc().
>>
>> You might consider the first patch in this series for stable.
>>
>> More details in the commit logs.
> 
> Series applied, honestly I don't think this is -stable material.
> 

Okay.

> And if it was you should have targetted 'net' instead of 'net-next'.
> 

Got it.

Thanks, Dave.
--
Gustavo