[PATCH][next] qtnfmac: replace qtnf_cmd_acl_data_size() with struct_size()

[PATCH][next] qtnfmac: replace qtnf_cmd_acl_data_size() with struct_size()

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

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 <gustavo@embeddedor.com>
---
 drivers/net/wireless/quantenna/qtnfmac/commands.c | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/drivers/net/wireless/quantenna/qtnfmac/commands.c b/drivers/net/wireless/quantenna/qtnfmac/commands.c
index 85a2a58f4c16..534d5a4d06fb 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/commands.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/commands.c
@@ -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,
-- 
2.21.0

Re: [PATCH][next] qtnfmac: replace qtnf_cmd_acl_data_size() with struct_size()

[Sergey Matyukevich]

> 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.
> 
> This code was detected with the help of Coccinelle.
> 
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> ---
>  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 <sergey.matyukevich.os@quantenna.com>

Regards,
Sergey

Re: [PATCH][next] qtnfmac: replace qtnf_cmd_acl_data_size() with struct_size()

[Gustavo A. R. Silva]

On 4/4/19 8:32 AM, Sergey Matyukevich wrote:
>> 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.
>>
>> This code was detected with the help of Coccinelle.
>>
>> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
>> ---
>>  drivers/net/wireless/quantenna/qtnfmac/commands.c | 15 ++++-----------
>>  1 file changed, 4 insertions(+), 11 deletions(-)
> 
> Hi Gustavo,
> 

Hey Sergey,

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

> Reviewed-by: Sergey Matyukevich <sergey.matyukevich.os@quantenna.com>
> 

I'll add your RB.

Thanks!
--
Gustavo

Re: [PATCH][next] qtnfmac: replace qtnf_cmd_acl_data_size() with struct_size()

[Gustavo A. R. Silva]

On 4/4/19 11:01 AM, Gustavo A. R. Silva wrote:
> 
> 
> On 4/4/19 8:32 AM, Sergey Matyukevich wrote:
>>> 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.
>>>
>>> This code was detected with the help of Coccinelle.
>>>
>>> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
>>> ---
>>>  drivers/net/wireless/quantenna/qtnfmac/commands.c | 15 ++++-----------
>>>  1 file changed, 4 insertions(+), 11 deletions(-)
>>
>> Hi Gustavo,
>>
> 
> Hey Sergey,
> 
>> 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

Re: [PATCH][next] qtnfmac: replace qtnf_cmd_acl_data_size() with struct_size()

[Kalle Valo]

"Gustavo A. R. Silva" <gustavo@embeddedor.com> wrote:

> 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.
> 
> This code was detected with the help of Coccinelle.
> 
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> Reviewed-by: Sergey Matyukevich <sergey.matyukevich.os@quantenna.com>

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

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

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

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

Re: [PATCH][next] qtnfmac: replace qtnf_cmd_acl_data_size() with struct_size()

[Kalle Valo]

"Gustavo A. R. Silva" <gustavo@embeddedor.com> writes:

> On 4/4/19 11:01 AM, Gustavo A. R. Silva wrote:
>> 
>> 
>> On 4/4/19 8:32 AM, Sergey Matyukevich wrote:
>>>> 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.
>>>>
>>>> This code was detected with the help of Coccinelle.
>>>>
>>>> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
>>>> ---
>>>>  drivers/net/wireless/quantenna/qtnfmac/commands.c | 15 ++++-----------
>>>>  1 file changed, 4 insertions(+), 11 deletions(-)
>>>
>>> Hi Gustavo,
>>>
>> 
>> Hey Sergey,
>> 
>>> 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?

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

-- 
Kalle Valo

Re: [PATCH][next] qtnfmac: replace qtnf_cmd_acl_data_size() with struct_size()

[Gustavo A. R. Silva]

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

[..]

>>>>
>>>> Hi Gustavo,
>>>>
>>>
>>> Hey Sergey,
>>>
>>>> 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?
> 
> Yeah, I also was able to apply it without problems. So it's in w-d-next
> now :)
> 

Awesome. :)

Thanks, Kalle.
--
Gustavo