* [PATCH][next] Bluetooth: a2mp: Use struct_size() helper
@ 2019-02-08 0:28 Gustavo A. R. Silva
2019-02-08 4:00 ` Joe Perches
2019-02-18 13:02 ` Marcel Holtmann
0 siblings, 2 replies; 5+ messages in thread
From: Gustavo A. R. Silva @ 2019-02-08 0:28 UTC (permalink / raw)
To: Marcel Holtmann, Johan Hedberg, David S. Miller
Cc: linux-bluetooth, netdev, linux-kernel, 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 = 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.
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
net/bluetooth/a2mp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/bluetooth/a2mp.c b/net/bluetooth/a2mp.c
index 58fc6333d412..5f918ea18b5a 100644
--- 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);
--
2.20.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH][next] Bluetooth: a2mp: Use struct_size() helper
2019-02-08 0:28 [PATCH][next] Bluetooth: a2mp: Use struct_size() helper Gustavo A. R. Silva
@ 2019-02-08 4:00 ` Joe Perches
2019-02-08 4:07 ` Gustavo A. R. Silva
2019-02-18 13:02 ` Marcel Holtmann
1 sibling, 1 reply; 5+ messages in thread
From: Joe Perches @ 2019-02-08 4:00 UTC (permalink / raw)
To: Gustavo A. R. Silva, Marcel Holtmann, Johan Hedberg, David S. Miller
Cc: linux-bluetooth, netdev, linux-kernel
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.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH][next] Bluetooth: a2mp: Use struct_size() helper
2019-02-08 4:00 ` Joe Perches
@ 2019-02-08 4:07 ` Gustavo A. R. Silva
0 siblings, 0 replies; 5+ messages in thread
From: Gustavo A. R. Silva @ 2019-02-08 4:07 UTC (permalink / raw)
To: Joe Perches, Marcel Holtmann, Johan Hedberg, David S. Miller
Cc: linux-bluetooth, netdev, linux-kernel
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH][next] Bluetooth: a2mp: Use struct_size() helper
2019-02-08 0:28 [PATCH][next] Bluetooth: a2mp: Use struct_size() helper Gustavo A. R. Silva
2019-02-08 4:00 ` Joe Perches
@ 2019-02-18 13:02 ` Marcel Holtmann
2019-02-18 17:50 ` Gustavo A. R. Silva
1 sibling, 1 reply; 5+ messages in thread
From: Marcel Holtmann @ 2019-02-18 13:02 UTC (permalink / raw)
To: Gustavo A. R. Silva
Cc: Johan Hedberg, David S. Miller, linux-bluetooth, netdev, linux-kernel
Hi Gustavo,
> 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.
>
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> ---
> net/bluetooth/a2mp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
patch has been applied to bluetooth-next tree.
Regards
Marcel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH][next] Bluetooth: a2mp: Use struct_size() helper
2019-02-18 13:02 ` Marcel Holtmann
@ 2019-02-18 17:50 ` Gustavo A. R. Silva
0 siblings, 0 replies; 5+ messages in thread
From: Gustavo A. R. Silva @ 2019-02-18 17:50 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Johan Hedberg, David S. Miller, linux-bluetooth, netdev, linux-kernel
On 2/18/19 7:02 AM, Marcel Holtmann wrote:
> Hi Gustavo,
>
>> 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.
>>
>> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
>> ---
>> net/bluetooth/a2mp.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> patch has been applied to bluetooth-next tree.
>
Thanks Marcel.
--
Gustavo
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-02-18 17:50 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-08 0:28 [PATCH][next] Bluetooth: a2mp: Use struct_size() helper Gustavo A. R. Silva
2019-02-08 4:00 ` Joe Perches
2019-02-08 4:07 ` Gustavo A. R. Silva
2019-02-18 13:02 ` Marcel Holtmann
2019-02-18 17:50 ` Gustavo A. R. Silva
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).