netlink: Return unsigned value for nla_len()
Subsystems (2)
PATCH v3 1/1 netlink: Return unsigned value for nla_len()
The return value from nla_len() is never expected to be negative, and can never be more than struct nlattr::nla_len (a u16). Adjust the prototype on the function. This will let GCC's value range optimization passes know that the return can never be negative, and can never be larger than u16. As recently discussed[1], this silences the following warning in GCC 12+:
net/wireless/nl80211.c: In function 'nl80211_set_cqm_rssi.isra':net/wireless/nl80211.c:12892:17: warning: 'memcpy' specified bound 18446744073709551615 exceeds maximum object size 9223372036854775807 [-Wstringop-overflow=]
12892 | memcpy(cqm_config->rssi_thresholds, thresholds,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
12893 | flex_array_size(cqm_config, rssi_thresholds,
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
12894 | n_thresholds));
| ~~~~~~~~~~~~~~A future change would be to clamp the subtraction to make sure it never wraps around if nla_len is somehow less than NLA_HDRLEN, which would have the additional benefit of being defensive in the face of nlattr corruption or logic errors.
---
v3: do not cast NLA_HDRLEN to u16 (nicolas.dichtel)
v2: https://lore.kernel.org/all/[email protected]/
v1: https://lore.kernel.org/all/[email protected]/
--- include/net/netlink.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
include/net/netlink.h +1 -1
@@ -1200,7 +1200,7 @@ static inline void *nla_data(const struct nlattr *nla)* nla_len - length of payload * @nla: netlink attribute */ -static inline int nla_len (const struct nlattr *nla) +static inline u16 nla_len (const struct nlattr *nla) {return nla->nla_len -NLA_HDRLEN ; }
Signature · 1 line
2.34.1
This patch was applied to netdev/net-next.git (main) by Jakub Kicinski <[email protected]>:
Hello:
This patch was applied to netdev/net-next.git (main) by Jakub Kicinski <[email protected]>:
Quoted 8 lines · On Wed, 6 Dec 2023 12:59:07 -0800 you wrote:
The return value from
nla_len()is never expected to be negative, and can never be more than structnlattr::nla_len(a u16). Adjust the prototype on the function. This will let GCC's value range optimization passes know that the return can never be negative, and can never be larger than u16. As recently discussed[1], this silences the following warning in GCC 12+:[...]
Here is the summary with links:
- [v3] netlink: Return unsigned value for nla_len()
https://git.kernel.org/netdev/net-next/c/172db56d90d2
You are awesome, thank you!
Signature · 2 lines
Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
From: Kees Cook
Quoted 9 lines
Sent: 06 December 2023 20:59
The return value from
nla_len()is never expected to be negative, and can never be more than structnlattr::nla_len(a u16). Adjust the prototype on the function. This will let GCC's value range optimization passes know that the return can never be negative, and can never be larger than u16. As recently discussed[1], this silences the following warning in GCC 12+:
...
include/net/netlink.h static inline void *nla_data(const struct nlattr *nla) · line 1206
Quoted 5 lines
include/net/netlink.h +1
-1-static inline int nla_len (const struct nlattr *nla) +static inline u16 nla_len (const struct nlattr *nla) {return nla->nla_len -NLA_HDRLEN ; }
It also adds an explicit mask with 0xffff.
I suspect that returning 'unsigned int' will silence the warning from gcc (since the error message has a huge max size).
If the value is too small copying ~64k or ~4G will both overflow the buffer.
The former might (just) be exploitable, the latter will crash (so is probably better!)
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)