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 <[email protected]>
> Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/ [1]
> Cc: Jakub Kicinski <[email protected]>
> Cc: "David S. Miller" <[email protected]>
> Cc: Eric Dumazet <[email protected]>
> Cc: Paolo Abeni <[email protected]>
> Cc: Johannes Berg <[email protected]>
> Cc: Jeff Johnson <[email protected]>
> Cc: Michael Walle <[email protected]>
> Cc: Max Schulze <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Kees Cook <[email protected]>
Looks good to me.
Reviewed-by: Gustavo A. R. Silva <[email protected]>
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;
> }
>
> /**