| Message ID | 20210402173617.895-1-phil@philpotter.co.uk (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: initialize local variables in net/ipv6/mcast.c and net/ipv4/igmp.c | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Guessed tree name to be net-next |
| netdev/subject_prefix | warning | Target tree name not specified in the subject |
| netdev/cc_maintainers | success | CCed 5 of 5 maintainers |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 5 this patch: 5 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 24 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 5 this patch: 5 |
| netdev/header_inline | success | Link |
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. > > 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; >
On Fri, Apr 02, 2021 at 07:49:44PM +0200, Eric Dumazet wrote: > > > 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. > 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: >> >> >> 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. >> > > 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: >> On Fri, Apr 02, 2021 at 07:49:44PM +0200, Eric Dumazet wrote: >>> >>> >>> 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. >>> >> >> 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: > > > > > > On 4/2/21 8:10 PM, Phillip Potter wrote: > >> On Fri, Apr 02, 2021 at 07:49:44PM +0200, Eric Dumazet wrote: > >>> > >>> > >>> 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. > >>> > >> > >> 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
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;
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(+)