All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] nl80211: prefer struct_size over open coded arithmetic
@ 2021-09-25 13:55 Len Baker
  0 siblings, 0 replies; only message in thread
From: Len Baker @ 2021-09-25 13:55 UTC (permalink / raw)
  To: Johannes Berg, David S. Miller, Jakub Kicinski
  Cc: Len Baker, Kees Cook, Gustavo A. R. Silva, linux-wireless,
	netdev, linux-hardening, linux-kernel

As noted in the "Deprecated Interfaces, Language Features, Attributes,
and Conventions" documentation [1], size calculations (especially
multiplication) should not be performed in memory allocator (or similar)
function arguments due to the risk of them overflowing. This could lead
to values wrapping around and a smaller allocation being made than the
caller was expecting. Using those allocations could lead to linear
overflows of heap memory and other misbehaviors.

So, use the struct_size() helper to do the arithmetic instead of the
argument "size + count * size" in the kzalloc() functions.

Also, take the opportunity to refactor the memcpy() call to use the
flex_array_size() helper.

This code was detected with the help of Coccinelle and audited and fixed
manually.

[1] https://www.kernel.org/doc/html/latest/process/deprecated.html#open-coded-arithmetic-in-allocator-arguments

Signed-off-by: Len Baker <len.baker@gmx.com>
---
Changelog v1 -> v2
- Rebase against v5.15-rc2
- Remove the unnecessary "size" variable (Gustavo A. R. Silva).
- Update the commit changelog to inform that this code was detected
  using a Coccinelle script (Gustavo A. R. Silva).

 net/wireless/nl80211.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index bf7cd4752547..fa7ff61c5b07 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -11767,8 +11767,8 @@ static int nl80211_set_cqm_rssi(struct genl_info *info,
 	if (n_thresholds) {
 		struct cfg80211_cqm_config *cqm_config;

-		cqm_config = kzalloc(sizeof(struct cfg80211_cqm_config) +
-				     n_thresholds * sizeof(s32), GFP_KERNEL);
+		cqm_config = kzalloc(struct_size(cqm_config, rssi_thresholds,
+						 n_thresholds), GFP_KERNEL);
 		if (!cqm_config) {
 			err = -ENOMEM;
 			goto unlock;
@@ -11777,7 +11777,8 @@ static int nl80211_set_cqm_rssi(struct genl_info *info,
 		cqm_config->rssi_hyst = hysteresis;
 		cqm_config->n_rssi_thresholds = n_thresholds;
 		memcpy(cqm_config->rssi_thresholds, thresholds,
-		       n_thresholds * sizeof(s32));
+		       flex_array_size(cqm_config, rssi_thresholds,
+				       n_thresholds));

 		wdev->cqm_config = cqm_config;
 	}
@@ -15081,9 +15082,7 @@ static int nl80211_set_sar_specs(struct sk_buff *skb, struct genl_info *info)
 	if (specs > rdev->wiphy.sar_capa->num_freq_ranges)
 		return -EINVAL;

-	sar_spec = kzalloc(sizeof(*sar_spec) +
-			   specs * sizeof(struct cfg80211_sar_sub_specs),
-			   GFP_KERNEL);
+	sar_spec = kzalloc(struct_size(sar_spec, sub_specs, specs), GFP_KERNEL);
 	if (!sar_spec)
 		return -ENOMEM;

--
2.25.1


^ permalink raw reply related	[flat|nested] only message in thread

only message in thread, other threads:[~2021-09-25 13:56 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-25 13:55 [PATCH v2] nl80211: prefer struct_size over open coded arithmetic Len Baker

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.