| Message ID | 20190403170236.GA31637@embeddedor (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 95336d4cb588860283047e01050ae41993e0147d |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | [next] qtnfmac: replace qtnf_cmd_acl_data_size() with struct_size() | expand |
> 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
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
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
"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()
"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 :)
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
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,
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(-)