qtnfmac: replace qtnf_cmd_acl_data_size() with struct_size()

PATCH In review Gustavo A. R. Silva 3 participants 7 messages Apr 3, 2019 – Apr 4, 2019

Subsystems (3)

Networking Network drivers Quantenna qtnfmac wireless driver

lore

Share & tools

lore

Gustavo A. R. Silva

author

2019-04-03 17:02 UTC #0 lore raw

PATCH 1/1 qtnfmac: replace qtnf_cmd_acl_data_size() with struct_size()

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 = kzalloc(size, GFP_KERNEL)

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

size = struct_size(instance, entry, count);

instance = kzalloc(struct_size(instance, entry, count), GFP_KERNEL)

Based on the above, replace qtnf_cmd_acl_data_size() with the new struct_size() helper.

This code was detected with the help of Coccinelle.

Signed-off-by Gustavo A. R. Silva <[email protected]>

---
 drivers/net/wireless/quantenna/qtnfmac/commands.c | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

drivers/net/wireless/quantenna/qtnfmac/commands.c +4 ~~-11~~

index 85a2a58f4c16..534d5a4d06fb 100644
@@ -177,14 +177,6 @@ static void qtnf_cmd_tlv_ie_set_add(struct sk_buff *cmd_skb, u8 frame_type,
 		memcpy(tlv->ie_data, buf, len);
 }
 
-static inline size_t qtnf_cmd_acl_data_size(const struct cfg80211_acl_data *acl)
-{
-	size_t size = sizeof(struct qlink_acl_data) +
-		      acl->n_acl_entries * sizeof(struct qlink_mac_address);
-
-	return size;
-}
-
 static bool qtnf_cmd_start_ap_can_fit(const struct qtnf_vif *vif,
 				      const struct cfg80211_ap_settings *s)
 {

@@ -203,7 +195,7 @@ static bool qtnf_cmd_start_ap_can_fit(const struct qtnf_vif *vif,
 
 	if (s->acl)
 		len += sizeof(struct qlink_tlv_hdr) +
-		       qtnf_cmd_acl_data_size(s->acl);
+		       struct_size(s->acl, mac_addrs, s->acl->n_acl_entries);
 
 	if (len > (sizeof(struct qlink_cmd) + QTNF_MAX_CMD_BUF_SIZE)) {
 		pr_err("VIF%u.%u: can not fit AP settings: %u\n",

@@ -310,7 +302,8 @@ int qtnf_cmd_send_start_ap(struct qtnf_vif *vif,
 	}
 
 	if (s->acl) {
-		size_t acl_size = qtnf_cmd_acl_data_size(s->acl);
+		size_t acl_size = struct_size(s->acl, mac_addrs,
+					      s->acl->n_acl_entries);
 		struct qlink_tlv_hdr *tlv =
 			skb_put(cmd_skb, sizeof(*tlv) + acl_size);

@@ -2592,7 +2585,7 @@ int qtnf_cmd_set_mac_acl(const struct qtnf_vif *vif,
 	struct qtnf_bus *bus = vif->mac->bus;
 	struct sk_buff *cmd_skb;
 	struct qlink_tlv_hdr *tlv;
-	size_t acl_size = qtnf_cmd_acl_data_size(params);
+	size_t acl_size = struct_size(params, mac_addrs, params->n_acl_entries);
 	int ret;
 
 	cmd_skb = qtnf_cmd_alloc_new_cmdskb(vif->mac->macid, vif->vifid,

Signature · 1 line

2.21.0

Sergey Matyukevich

Reviewed-by on v1 1/1

2019-04-04 13:33 UTC #1 lore raw

Quoted 24 lines

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 = kzalloc(size, GFP_KERNEL)
Instead of leaving these open-coded and prone to type mistakes, we can now use the new struct_size() helper:
size = struct_size(instance, entry, count);
or
instance = kzalloc(struct_size(instance, entry, count), GFP_KERNEL)
Based on the above, replace qtnf_cmd_acl_data_size() with the new struct_size() helper.

Quoted 6 lines

This code was detected with the help of Coccinelle.
Signed-off-by Gustavo A. R. Silva <[email protected]>
---
 drivers/net/wireless/quantenna/qtnfmac/commands.c | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

Hi Gustavo,

Thanks for the patch! By the way, it does not apply cleanly, so it needs to be rebased on top of the up-to-date wireless-drivers-next tree. Let me know if you would prefer me to care about rebase. Then I will add this patch to the upcoming series of qtnfmac fixes.

Reviewed-by Sergey Matyukevich <[email protected]>

Regards,
Sergey

Gustavo A. R. Silva

author

2019-04-04 16:01 UTC #2 lore raw

Quoted 24 lines · Sergey Matyukevich

Quoted 24 lines
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 = kzalloc(size, GFP_KERNEL)
Instead of leaving these open-coded and prone to type mistakes, we can now use the new struct_size() helper:
size = struct_size(instance, entry, count);
or
instance = kzalloc(struct_size(instance, entry, count), GFP_KERNEL)
Based on the above, replace qtnf_cmd_acl_data_size() with the new struct_size() helper.

Quoted 9 lines

Quoted 6 lines
This code was detected with the help of Coccinelle.
Signed-off-by Gustavo A. R. Silva <[email protected]>
---
 drivers/net/wireless/quantenna/qtnfmac/commands.c | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)
Hi Gustavo,

Hey Sergey,

Quoted 5 lines

Thanks for the patch! By the way, it does not apply cleanly, so it needs to be rebased on top of the up-to-date wireless-drivers-next tree. Let me know if you would prefer me to care about rebase. Then I will add this patch to the upcoming series of qtnfmac fixes.

Don't worry. I'll do it and send this again.

Quoted 2 lines

Reviewed-by Sergey Matyukevich <[email protected]>

I'll add your RB.

Thanks!
--
Gustavo

Gustavo A. R. Silva

author

2019-04-04 16:36 UTC #3 lore raw

Quoted 39 lines · Gustavo A. R. Silva

Quoted 24 lines · Sergey Matyukevich
Quoted 24 lines
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 = kzalloc(size, GFP_KERNEL)
Instead of leaving these open-coded and prone to type mistakes, we can now use the new struct_size() helper:
size = struct_size(instance, entry, count);
or
instance = kzalloc(struct_size(instance, entry, count), GFP_KERNEL)
Based on the above, replace qtnf_cmd_acl_data_size() with the new struct_size() helper.
Quoted 9 lines
Quoted 6 lines
This code was detected with the help of Coccinelle.
Signed-off-by Gustavo A. R. Silva <[email protected]>
---
 drivers/net/wireless/quantenna/qtnfmac/commands.c | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)
Hi Gustavo,
Hey Sergey,

Quoted 8 lines

Quoted 5 lines
Thanks for the patch! By the way, it does not apply cleanly, so it needs to be rebased on top of the up-to-date wireless-drivers-next tree. Let me know if you would prefer me to care about rebase. Then I will add this patch to the upcoming series of qtnfmac fixes.
Don't worry. I'll do it and send this again.

Hmm... I just applied it cleanly on top of wireless-drivers-next/master:

973a99be7943 (HEAD) qtnfmac: replace qtnf_cmd_acl_data_size() with struct_size() 38bb0baea310 (wireless-drivers-next/master) rtlwifi: move spin_lock_bh to spin_lock in tasklet 60209d482b97 rtlwifi: fix potential NULL pointer dereference
765976285a8c rtlwifi: fix a potential NULL pointer dereference

Do you see any issues with this?

Thanks
--
Gustavo

Kalle Valo

2019-04-04 16:39 UTC #6 lore raw

In reply to Gustavo A. R. Silva

Quoted 21 lines · 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 = kzalloc(size, GFP_KERNEL)
Instead of leaving these open-coded and prone to type mistakes, we can now use the new struct_size() helper:
size = struct_size(instance, entry, count);
or
instance = kzalloc(struct_size(instance, entry, count), GFP_KERNEL)

Quoted 7 lines

Based on the above, replace qtnf_cmd_acl_data_size() with the new struct_size() helper.
This code was detected with the help of Coccinelle.
Signed-off-by Gustavo A. R. Silva <[email protected]>
Reviewed-by Sergey Matyukevich <[email protected]>

Patch applied to wireless-drivers-next.git, thanks.

95336d4cb588 qtnfmac: replace qtnf_cmd_acl_data_size() with struct_size()

Signature · 3 lines

https://patchwork.kernel.org/patch/10884311/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Kalle Valo

2019-04-04 16:40 UTC #4 lore raw

In reply to Gustavo A. R. Silva

Quoted 51 lines · Gustavo A. R. Silva

Quoted 39 lines · Gustavo A. R. Silva
Quoted 24 lines · Sergey Matyukevich
Quoted 24 lines
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 = kzalloc(size, GFP_KERNEL)
Instead of leaving these open-coded and prone to type mistakes, we can now use the new struct_size() helper:
size = struct_size(instance, entry, count);
or
instance = kzalloc(struct_size(instance, entry, count), GFP_KERNEL)
Based on the above, replace qtnf_cmd_acl_data_size() with the new struct_size() helper.
Quoted 9 lines
Quoted 6 lines
This code was detected with the help of Coccinelle.
Signed-off-by Gustavo A. R. Silva <[email protected]>
---
 drivers/net/wireless/quantenna/qtnfmac/commands.c | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)
Hi Gustavo,
Hey Sergey,
Quoted 8 lines
Quoted 5 lines
Thanks for the patch! By the way, it does not apply cleanly, so it needs to be rebased on top of the up-to-date wireless-drivers-next tree. Let me know if you would prefer me to care about rebase. Then I will add this patch to the upcoming series of qtnfmac fixes.
Don't worry. I'll do it and send this again.
Hmm... I just applied it cleanly on top of wireless-drivers-next/master:

Quoted 7 lines

973a99be7943 (HEAD) qtnfmac: replace qtnf_cmd_acl_data_size() with struct_size() 38bb0baea310 (wireless-drivers-next/master) rtlwifi: move spin_lock_bh to spin_lock in tasklet
60209d482b97 rtlwifi: fix potential NULL pointer dereference 765976285a8c rtlwifi: fix a potential NULL pointer dereference
Do you see any issues with this?

Yeah, I also was able to apply it without problems. So it's in w-d-next now :)

Signature · 1 line

Kalle Valo

Gustavo A. R. Silva

author

2019-04-04 16:46 UTC #5 lore raw

On 4/4/19 11:40 AM, Kalle Valo wrote:

[..]

Quoted 23 lines

Quoted 23 lines
Quoted 6 lines
Quoted 3 lines
Hi Gustavo,
Hey Sergey,
Quoted 8 lines
Quoted 5 lines
Thanks for the patch! By the way, it does not apply cleanly, so it needs to be rebased on top of the up-to-date wireless-drivers-next tree. Let me know if you would prefer me to care about rebase. Then I will add this patch to the upcoming series of qtnfmac fixes.
Don't worry. I'll do it and send this again.
Hmm... I just applied it cleanly on top of wireless-drivers-next/master:
973a99be7943 (HEAD) qtnfmac: replace qtnf_cmd_acl_data_size() with struct_size() 38bb0baea310 (wireless-drivers-next/master) rtlwifi: move spin_lock_bh to spin_lock in tasklet
60209d482b97 rtlwifi: fix potential NULL pointer dereference 765976285a8c rtlwifi: fix a potential NULL pointer dereference

Quoted 5 lines

Quoted 1 line
Do you see any issues with this?
Yeah, I also was able to apply it without problems. So it's in w-d-next now :)

Awesome. :)

Thanks, Kalle.
--
Gustavo