Messages in this thread | | | Subject | Re: [PATCH][next] Bluetooth: a2mp: Use struct_size() helper | From | "Gustavo A. R. Silva" <> | Date | Thu, 7 Feb 2019 22:07:10 -0600 |
| |
On 2/7/19 10:00 PM, Joe Perches wrote: > On Thu, 2019-02-07 at 18:28 -0600, Gustavo A. R. Silva 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 = alloc(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 = alloc(size, GFP_KERNEL) >> >> This code was detected with the help of Coccinelle. > [] >> diff --git a/net/bluetooth/a2mp.c b/net/bluetooth/a2mp.c > [] >> @@ -174,7 +174,7 @@ static int a2mp_discover_req(struct amp_mgr *mgr, struct sk_buff *skb, >> num_ctrl++; >> } >> >> - len = num_ctrl * sizeof(struct a2mp_cl) + sizeof(*rsp); >> + len = struct_size(rsp, cl, num_ctrl); >> rsp = kmalloc(len, GFP_ATOMIC); >> if (!rsp) { >> read_unlock(&hci_dev_list_lock); > > At least a weakness in this code is len is u16 > and struct_size is size_t so there's a size > truncation possible. > >
That's true. I didn't change the type to size_t because of the call to le16_to_cpu():
u16 len = le16_to_cpu(hdr->len);
I've been changing the type of the variable in other cases.
-- Gustavo
| |