On Thu, 2022-06-02 at 09:02 -0700, Jakub Kicinski wrote: > On Thu, 02 Jun 2022 12:38:10 +0200 Paolo Abeni wrote: > > I'm sorry for the multiple incremental feedback on this patch. It's > > somewhat tricky. > > > > AFAICS Jakub mentioned only udpv6_sendmsg(). In l2tp_ip6_sendmsg() we > > can have an overflow: > > > > int transhdrlen = 4; /* zero session-id */ > > int ulen = len + transhdrlen; > > > > when len >= INT_MAX - 4. That will be harmless, but I guess it could > > still trigger a noisy UBSAN splat. > > Good point, I wonder if that's a separate issue. Should we > follow what UDP does and subtract the transhdr from the max? > My gut feeling is that stricter checks are cleaner than just > bumping variable sizes. > > diff --git a/net/l2tp/l2tp_ip6.c b/net/l2tp/l2tp_ip6.c > index c6ff8bf9b55f..9dbd801ddb98 100644 > --- a/net/l2tp/l2tp_ip6.c > +++ b/net/l2tp/l2tp_ip6.c > @@ -504,14 +504,15 @@ static int l2tp_ip6_sendmsg(struct sock *sk, struct msghdr *msg, size_t len) > struct ipcm6_cookie ipc6; > int addr_len = msg->msg_namelen; > int transhdrlen = 4; /* zero session-id */ > - int ulen = len + transhdrlen; > + int ulen; > int err; > > /* Rough check on arithmetic overflow, > * better check is made in ip6_append_data(). > */ > - if (len > INT_MAX) > + if (len > INT_MAX - transhdrlen) > return -EMSGSIZE; > + ulen = len + transhdrlen; > > /* Mirror BSD error message compatibility */ > if (msg->msg_flags & MSG_OOB) > LGTM. Imho this can even land in a separated patch (whatever is easier) Thanks! Paolo
