From: Jakub Kicinski <kuba@kernel.org>
To: Paolo Abeni <pabeni@redhat.com>
Cc: Wang Yufen <wangyufen@huawei.com>,
davem@davemloft.net, yoshfuji@linux-ipv6.org, dsahern@kernel.org,
edumazet@google.com, ast@kernel.org, daniel@iogearbox.net,
andrii@kernel.org, kafai@fb.com, songliubraving@fb.com,
yhs@fb.com, john.fastabend@gmail.com, kpsingh@kernel.org,
netdev@vger.kernel.org, bpf@vger.kernel.org
Subject: Re: [PATCH net-next v4] ipv6: Fix signed integer overflow in __ip6_append_data
Date: Thu, 2 Jun 2022 09:02:28 -0700 [thread overview]
Message-ID: <20220602090228.1e493e47@kernel.org> (raw)
In-Reply-To: <4c9e3cf5122f4c2f8a2473c493891362e0a13b4a.camel@redhat.com>
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)
next prev parent reply other threads:[~2022-06-02 16:02 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-06-01 8:48 [PATCH net-next v4] ipv6: Fix signed integer overflow in __ip6_append_data Wang Yufen
2022-06-02 10:38 ` Paolo Abeni
2022-06-02 16:02 ` Jakub Kicinski [this message]
2022-06-03 8:58 ` Paolo Abeni
2022-06-06 2:03 ` wangyufen
2022-06-06 14:24 ` Jakub Kicinski
2022-06-07 1:16 ` wangyufen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220602090228.1e493e47@kernel.org \
--to=kuba@kernel.org \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=dsahern@kernel.org \
--cc=edumazet@google.com \
--cc=john.fastabend@gmail.com \
--cc=kafai@fb.com \
--cc=kpsingh@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=songliubraving@fb.com \
--cc=wangyufen@huawei.com \
--cc=yhs@fb.com \
--cc=yoshfuji@linux-ipv6.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.