Skip to contentSkip to conversation

Bluetooth: Use struct_size() helper

PATCH In review Gustavo A. R. Silva 3 participants 4 messages Mar 28, 2019 – Apr 23, 2019

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Bluetooth: Use struct_size() helper

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.

---
 net/bluetooth/mgmt.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)
net/bluetooth/mgmt.c +4 -7
index 2457f408d17d..150114e33b20 100644
@@ -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);
Signature · 1 line
2.21.0
#1 lore raw
Quoted 9 lines · 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[];
};
Quoted 6 lines

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

[]

net/bluetooth/mgmt.c

Quoted 1 line
net/bluetooth/mgmt.c +0 -0

[]

Quoted 30 lines
net/bluetooth/mgmt.c +3 -5
@@ -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);

net/bluetooth/mgmt.c static int load_conn_param(struct sock *sk, struct hci_dev *hdev, void *data, · line 5851

Quoted 9 lines
net/bluetooth/mgmt.c +1 -2
@@ -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...

maintainer
#2 lore raw
In reply to Gustavo A. R. Silva

Hi Gustavo,

Quoted 14 lines

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:

Quoted 8 lines

size = struct_size(instance, entry, count);

This code was detected with the help of Coccinelle.

---
net/bluetooth/mgmt.c | 11 ++++-------

1 file changed, 4 insertions(+), 7 deletions(-)

patch has been applied to bluetooth-next tree.

Regards

Marcel

#3 lore raw
Quoted 2 lines · Marcel Holtmann

Hi Gustavo,

[..]

Quoted 8 lines
Quoted 5 lines

---
net/bluetooth/mgmt.c | 11 ++++-------

1 file changed, 4 insertions(+), 7 deletions(-)

patch has been applied to bluetooth-next tree.

Awesome.

Thanks, Marcel.
--
Gustavo