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 <[email protected]>
---
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
> 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 <[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
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 <[email protected]>
>> ---
>> 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 <[email protected]>
>
I'll add your RB.
Thanks!
--
Gustavo
"Gustavo A. R. Silva" <[email protected]> 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 <[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()
--
https://patchwork.kernel.org/patch/10884311/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
"Gustavo A. R. Silva" <[email protected]> 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 <[email protected]>
>>>> ---
>>>> 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
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 <[email protected]>
>>> ---
>>> 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
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