On Sat, Nov 04, 2023 at 11:01:54PM +0000, Jeremy Sowden wrote:
> The `len` parameter of `mnl_nlmsg_ok`, which holds the buffer length and
> is compared to the size of the object expected to fit into the buffer,
> is signed because the function validates the length, and it can be
> negative in the case of malformed messages. Comparing it to unsigned
> operands used to lead to compiler warnings:
>
> msg.c: In function 'mnl_nlmsg_ok':
> msg.c:136: warning: comparison between signed and unsigned
> msg.c:138: warning: comparison between signed and unsigned
>
> and so commit 73661922bc3b ("fix warning in compilation due to different
> signess") added casts of the unsigned operands to `int`. However, the
> comparison to `nlh->nlmsg_len`:
>
> (int)nlh->nlmsg_len <= len
>
> is problematic, since `nlh->nlmsg_len` is of type `__u32` and so may
> hold values greater than `INT_MAX`. In the case where `len` is positive
> and `nlh->nlmsg_len` is greater than `INT_MAX`, the cast will yield a
> negative value and `mnl_nlmsg_ok` will incorrectly return true.
>
> Instead, assign `len` to an unsigned local variable, check for a
> negative value first, then use the unsigned local for the other
> comparisons, and remove the casts.
Applied, thanks Jeremy
