[PATCH] net: initialize local variables in net/ipv6/mcast.c and net/ipv4/igmp.c

Use memset to initialize two local buffers in net/ipv6/mcast.c,
and another in net/ipv4/igmp.c. Fixes a KMSAN found uninit-value
bug reported by syzbot at:
https://syzkaller.appspot.com/bug?id=0766d38c656abeace60621896d705743aeefed51

Reported-by: syzbot+001516d86dbe88862cec@syzkaller.appspotmail.com
Signed-off-by: Phillip Potter <phil@philpotter.co.uk>
---
 net/ipv4/igmp.c  | 2 ++
 net/ipv6/mcast.c | 4 ++++
 2 files changed, 6 insertions(+)

diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
index 7b272bbed2b4..bc8e358a9a2a 100644
--- a/net/ipv4/igmp.c
+++ b/net/ipv4/igmp.c

@@ -1131,6 +1131,8 @@ static void ip_mc_filter_add(struct in_device *in_dev, __be32 addr)
 	char buf[MAX_ADDR_LEN];
 	struct net_device *dev = in_dev->dev;
 
+	memset(buf, 0, sizeof(buf));
+
 	/* Checking for IFF_MULTICAST here is WRONG-WRONG-WRONG.
 	   We will get multicast token leakage, when IFF_MULTICAST
 	   is changed. This check should be done in ndo_set_rx_mode

diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index 6c8604390266..ad90dc28f318 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c

@@ -658,6 +658,8 @@ static void igmp6_group_added(struct ifmcaddr6 *mc)
 	struct net_device *dev = mc->idev->dev;
 	char buf[MAX_ADDR_LEN];
 
+	memset(buf, 0, sizeof(buf));
+
 	if (IPV6_ADDR_MC_SCOPE(&mc->mca_addr) <
 	    IPV6_ADDR_SCOPE_LINKLOCAL)
 		return;

@@ -694,6 +696,8 @@ static void igmp6_group_dropped(struct ifmcaddr6 *mc)
 	struct net_device *dev = mc->idev->dev;
 	char buf[MAX_ADDR_LEN];
 
+	memset(buf, 0, sizeof(buf));
+
 	if (IPV6_ADDR_MC_SCOPE(&mc->mca_addr) <
 	    IPV6_ADDR_SCOPE_LINKLOCAL)
 		return;

-- 
2.30.2

On 4/2/21 7:36 PM, Phillip Potter wrote:

Use memset to initialize two local buffers in net/ipv6/mcast.c,
and another in net/ipv4/igmp.c. Fixes a KMSAN found uninit-value
bug reported by syzbot at:
https://syzkaller.appspot.com/bug?id=0766d38c656abeace60621896d705743aeefed51

According to this link, the bug no longer triggers.

Please explain why you think it is still there.

On Fri, Apr 02, 2021 at 07:49:44PM +0200, Eric Dumazet wrote:

On 4/2/21 7:36 PM, Phillip Potter wrote:

quoted

Use memset to initialize two local buffers in net/ipv6/mcast.c,
and another in net/ipv4/igmp.c. Fixes a KMSAN found uninit-value
bug reported by syzbot at:
https://syzkaller.appspot.com/bug?id=0766d38c656abeace60621896d705743aeefed51

According to this link, the bug no longer triggers.

Please explain why you think it is still there.

Dear Eric,

It definitely still triggers, tested it on the master branch of
https://github.com/google/kmsan last night. The patch which fixes the
crash on that page is the same patch I've sent in.

Regards,
Phil

On 4/2/21 8:10 PM, Phillip Potter wrote:

On Fri, Apr 02, 2021 at 07:49:44PM +0200, Eric Dumazet wrote:

quoted

On 4/2/21 7:36 PM, Phillip Potter wrote:

quoted

Use memset to initialize two local buffers in net/ipv6/mcast.c,
and another in net/ipv4/igmp.c. Fixes a KMSAN found uninit-value
bug reported by syzbot at:
https://syzkaller.appspot.com/bug?id=0766d38c656abeace60621896d705743aeefed51

According to this link, the bug no longer triggers.

Please explain why you think it is still there.

Dear Eric,

It definitely still triggers, tested it on the master branch of
https://github.com/google/kmsan last night. The patch which fixes the
crash on that page is the same patch I've sent in.

Please send the full report (stack trace)

Thanks.

On 4/2/21 10:53 PM, Eric Dumazet wrote:

On 4/2/21 8:10 PM, Phillip Potter wrote:

quoted

On Fri, Apr 02, 2021 at 07:49:44PM +0200, Eric Dumazet wrote:

quoted

On 4/2/21 7:36 PM, Phillip Potter wrote:

quoted

Use memset to initialize two local buffers in net/ipv6/mcast.c,
and another in net/ipv4/igmp.c. Fixes a KMSAN found uninit-value
bug reported by syzbot at:
https://syzkaller.appspot.com/bug?id=0766d38c656abeace60621896d705743aeefed51

According to this link, the bug no longer triggers.

Please explain why you think it is still there.

Dear Eric,

It definitely still triggers, tested it on the master branch of
https://github.com/google/kmsan last night. The patch which fixes the
crash on that page is the same patch I've sent in.

Please send the full report (stack trace)

I think your patch just silences the real problem.

The issue at hand is that TUNSETLINK changes dev->type without making
any change to dev->addr_len

This is the real issue.

If you care about this, please fix tun driver.

On Fri, Apr 02, 2021 at 11:12:36PM +0200, Eric Dumazet wrote:

On 4/2/21 10:53 PM, Eric Dumazet wrote:

quoted

On 4/2/21 8:10 PM, Phillip Potter wrote:

quoted

On Fri, Apr 02, 2021 at 07:49:44PM +0200, Eric Dumazet wrote:

quoted

On 4/2/21 7:36 PM, Phillip Potter wrote:

quoted

Use memset to initialize two local buffers in net/ipv6/mcast.c,
and another in net/ipv4/igmp.c. Fixes a KMSAN found uninit-value
bug reported by syzbot at:
https://syzkaller.appspot.com/bug?id=0766d38c656abeace60621896d705743aeefed51

According to this link, the bug no longer triggers.

Please explain why you think it is still there.

Dear Eric,

It definitely still triggers, tested it on the master branch of
https://github.com/google/kmsan last night. The patch which fixes the
crash on that page is the same patch I've sent in.

Please send the full report (stack trace)

I think your patch just silences the real problem.

The issue at hand is that TUNSETLINK changes dev->type without making
any change to dev->addr_len

This is the real issue.

If you care about this, please fix tun driver.

Dear Eric,

Thank you for pointing me in the right direction. I will do as you
suggest.

Regards,
Phil