[PATCH][next] Bluetooth: Use struct_size() helper

[PATCH][next] Bluetooth: Use struct_size() helper

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

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

This code was detected with the help of Coccinelle.

Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
 net/bluetooth/mgmt.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 2457f408d17d..150114e33b20 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2301,8 +2301,7 @@ static int load_link_keys(struct sock *sk, struct hci_dev *hdev, void *data,
 				       MGMT_STATUS_INVALID_PARAMS);
 	}
 
-	expected_len = sizeof(*cp) + key_count *
-					sizeof(struct mgmt_link_key_info);
+	expected_len = struct_size(cp, keys, key_count);
 	if (expected_len != len) {
 		bt_dev_err(hdev, "load_link_keys: expected %u bytes, got %u bytes",
 			   expected_len, len);
@@ -5030,7 +5029,7 @@ static int load_irks(struct sock *sk, struct hci_dev *hdev, void *cp_data,
 				       MGMT_STATUS_INVALID_PARAMS);
 	}
 
-	expected_len = sizeof(*cp) + irk_count * sizeof(struct mgmt_irk_info);
+	expected_len = struct_size(cp, irks, irk_count);
 	if (expected_len != len) {
 		bt_dev_err(hdev, "load_irks: expected %u bytes, got %u bytes",
 			   expected_len, len);
@@ -5112,8 +5111,7 @@ static int load_long_term_keys(struct sock *sk, struct hci_dev *hdev,
 				       MGMT_STATUS_INVALID_PARAMS);
 	}
 
-	expected_len = sizeof(*cp) + key_count *
-					sizeof(struct mgmt_ltk_info);
+	expected_len = struct_size(cp, keys, key_count);
 	if (expected_len != len) {
 		bt_dev_err(hdev, "load_keys: expected %u bytes, got %u bytes",
 			   expected_len, len);
@@ -5847,8 +5845,7 @@ static int load_conn_param(struct sock *sk, struct hci_dev *hdev, void *data,
 				       MGMT_STATUS_INVALID_PARAMS);
 	}
 
-	expected_len = sizeof(*cp) + param_count *
-					sizeof(struct mgmt_conn_param);
+	expected_len = struct_size(cp, params, param_count);
 	if (expected_len != len) {
 		bt_dev_err(hdev, "load_conn_param: expected %u bytes, got %u bytes",
 			   expected_len, len);
-- 
2.21.0

Re: [PATCH][next] Bluetooth: Use struct_size() helper

[Joe Perches]

On Thu, 2019-03-28 at 12:30 -0500, 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);
> 
> 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);
[]
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
[]
> @@ -2301,8 +2301,7 @@ static int load_link_keys(struct sock *sk, struct hci_dev *hdev, void *data,
>  				       MGMT_STATUS_INVALID_PARAMS);
>  	}
>  
> -	expected_len = sizeof(*cp) + key_count *
> -					sizeof(struct mgmt_link_key_info);
> +	expected_len = struct_size(cp, keys, key_count);
>  	if (expected_len != len) {
>  		bt_dev_err(hdev, "load_link_keys: expected %u bytes, got %u bytes",
>  			   expected_len, len);
> @@ -5030,7 +5029,7 @@ static int load_irks(struct sock *sk, struct hci_dev *hdev, void *cp_data,
>  				       MGMT_STATUS_INVALID_PARAMS);
>  	}
>  
> -	expected_len = sizeof(*cp) + irk_count * sizeof(struct mgmt_irk_info);
> +	expected_len = struct_size(cp, irks, irk_count);
>  	if (expected_len != len) {
>  		bt_dev_err(hdev, "load_irks: expected %u bytes, got %u bytes",
>  			   expected_len, len);
> @@ -5112,8 +5111,7 @@ static int load_long_term_keys(struct sock *sk, struct hci_dev *hdev,
>  				       MGMT_STATUS_INVALID_PARAMS);
>  	}
>  
> -	expected_len = sizeof(*cp) + key_count *
> -					sizeof(struct mgmt_ltk_info); 
> +	expected_len = struct_size(cp, keys, key_count);
>  	if (expected_len != len) {
>  		bt_dev_err(hdev, "load_keys: expected %u bytes, got %u bytes",
>  			   expected_len, len);
> @@ -5847,8 +5845,7 @@ static int load_conn_param(struct sock *sk, struct hci_dev *hdev, void *data,
>  				       MGMT_STATUS_INVALID_PARAMS);
>  	}
>  
> -	expected_len = sizeof(*cp) + param_count *
> -					sizeof(struct mgmt_conn_param);
> +	expected_len = struct_size(cp, params, param_count);
>  	if (expected_len != len) {
>  		bt_dev_err(hdev, "load_conn_param: expected %u bytes, got %u bytes",
>  			   expected_len, len);

Maybe add a helper function for all of these.
Perhaps something like:

static bool verify_len(size_t actual, size_t expected)
{
	if (actual == expected)
		return true;
	bt_dev_err("%ps: expected %zu bytes, got %zu bytes",
		   __builtin_return_address(1), expected, actual);
	return false;
}

So the last block above might be

	if (!verify_len(len, struct_size(cp, params, param_count),))
		return mgmt_cmd_status(sk, hdev->id,  MGMT_OP_LOAD_CONN_PARAM,
				       MGMT_STATUS_INVALID_PARAMS);

etc...

Re: [PATCH][next] Bluetooth: Use struct_size() helper

[Marcel Holtmann]

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);
> 
> 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);
> 
> This code was detected with the help of Coccinelle.
> 
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> ---
> net/bluetooth/mgmt.c | 11 ++++-------
> 1 file changed, 4 insertions(+), 7 deletions(-)

patch has been applied to bluetooth-next tree.

Regards

Marcel

Re: [PATCH][next] Bluetooth: Use struct_size() helper

[Gustavo A. R. Silva]

On 4/23/19 12:31 PM, Marcel Holtmann wrote:
> Hi Gustavo,
> 
[..]
>>
>> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
>> ---
>> net/bluetooth/mgmt.c | 11 ++++-------
>> 1 file changed, 4 insertions(+), 7 deletions(-)
> 
> patch has been applied to bluetooth-next tree.
> 

Awesome.

Thanks, Marcel.
--
Gustavo