Search Linux Wireless

Re: [PATCH] netlink: Return unsigned value for nla_len()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 11/30/23 14:01, Kees Cook wrote:
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, and explicitly bounds check the subtraction. 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));
       |                                        ~~~~~~~~~~~~~~

This has the additional benefit of being defensive in the face of nlattr
corruption or logic errors (i.e. nla_len being set smaller than
NLA_HDRLEN).

Reported-by: kernel test robot <lkp@xxxxxxxxx>
Closes: https://lore.kernel.org/oe-kbuild-all/202311090752.hWcJWAHL-lkp@xxxxxxxxx/ [1]
Cc: Jakub Kicinski <kuba@xxxxxxxxxx>
Cc: "David S. Miller" <davem@xxxxxxxxxxxxx>
Cc: Eric Dumazet <edumazet@xxxxxxxxxx>
Cc: Paolo Abeni <pabeni@xxxxxxxxxx>
Cc: Johannes Berg <johannes@xxxxxxxxxxxxxxxx>
Cc: Jeff Johnson <quic_jjohnson@xxxxxxxxxxx>
Cc: Michael Walle <mwalle@xxxxxxxxxx>
Cc: Max Schulze <max.schulze@xxxxxxxxx>
Cc: netdev@xxxxxxxxxxxxxxx
Cc: linux-wireless@xxxxxxxxxxxxxxx
Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx>

Looks good to me.

Reviewed-by: Gustavo A. R. Silva <gustavoars@xxxxxxxxxx>

Thanks
--
Gustavo

---
  include/net/netlink.h | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/net/netlink.h b/include/net/netlink.h
index 167b91348e57..c59679524705 100644
--- a/include/net/netlink.h
+++ b/include/net/netlink.h
@@ -1214,9 +1214,9 @@ 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;
+	return nla->nla_len > NLA_HDRLEN ? nla->nla_len - NLA_HDRLEN : 0;
  }
/**




[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Wireless Personal Area Network]     [Linux Bluetooth]     [Wireless Regulations]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Hiking]     [MIPS Linux]     [ARM Linux]     [Linux RAID]

  Powered by Linux