On Wed, Aug 31, 2022 at 08:18:25PM -0700, Jakub Kicinski wrote:
> On Wed, 31 Aug 2022 20:06:09 -0700 Kees Cook wrote:
> > static inline int nlmsg_len(const struct nlmsghdr *nlh)
> > {
> > - return nlh->nlmsg_len - NLMSG_HDRLEN;
> > + u32 nlmsg_contents_len;
> > +
> > + if (WARN_ON_ONCE(check_sub_overflow(nlh->nlmsg_len,
> > + (u32)NLMSG_HDRLEN,
> > + &nlmsg_contents_len)))
> > + return 0;
> > + if (WARN_ON_ONCE(nlmsg_contents_len > INT_MAX))
> > + return INT_MAX;
> > + return nlmsg_contents_len;
>
> We check the messages on input, making sure the length is valid wrt
> skb->len, and sane, ie > NLMSG_HDRLEN. See netlink_rcv_skb().
>
> Can we not, pretty please? :(
This would catch corrupted values...
Is the concern the growth in image size? The check_sub_overflow() isn't
large at all -- it's just adding a single overflow bit test. The WARNs
are heavier, but they're all out-of-line.
--
Kees Cook
