| Message ID | 4A2D006C.70302@cosmosbay.com |
|---|---|
| State | Superseded, archived |
| Delegated to: | David Miller |
| Headers | show |
Mon, Jun 08, 2009 at 02:13:32PM CEST, dada1@cosmosbay.com wrote: >Eric Dumazet a écrit : >> Vegard Nossum a écrit : >>> 2009/6/7 John Dykstra <john.dykstra1@gmail.com>: >>>> On Sat, 2009-05-30 at 22:23 +0200, Vegard Nossum wrote: >>>>> It seems that loopback's hardware address is never initialized by the >>>>> kernel. So if userspace attempts to read this address before it has >>>>> been set, the kernel will return some uninitialized data (only 6 >>>>> bytes, though). >>>> Thank you for the report, Vegard. >>>> >>>> I've been unable to reproduce the problem you describe, using >>>> 2.6-30-rc8, this test program and a couple of kernel builds for system >>>> load: >>> [...] >>>> ------------------------------------------------------------------ >>>> >>>> Looking at the kernel code, it appears that all bytes of struct >>>> net_device, including the L2 address, are initialized to zeros at >>>> interface creation time. >>>> >>>> Can you spot a difference between your test procedures and mine that >>>> would enable me to reproduce the problem? >>> Hi, >>> >>> I just tried your test program on a linux-next kernel, it works beautifully :-) >>> >>> (I made one change: The stack grows downwards on x86, so I think you >>> should put child_stack + 16386 as the stack to clone()?) >>> >>> As I wrote in reply to Stephen Hemminger, this problem seems to be >>> caused by a particular patch in linux-next: >>> >>> commit f001fde5eadd915f4858d22ed70d7040f48767cf >>> Author: Jiri Pirko <jpirko@redhat.com> >>> Date: Tue May 5 02:48:28 2009 +0000 >>> >>> net: introduce a list of device addresses dev_addr_list (v6) >>> >> >> I believe following patch should fix this problem. >> >> Thank you >> >> [PATCH net-next-2.6] net: loopback device dev->addr_len fix >> >> commit f001fde5eadd915f4858d22ed70d7040f48767cf >> (net: introduce a list of device addresses dev_addr_list (v6)) >> added one regression Vegard Nossum found in its testings. >> >> loopback device doesnt have a hw address, we should set its >> dev->addr_len to 0, not ETH_ALEN. >> >> Reported-by: Vegard Nossum <vegard.nossum@gmail.com> >> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> oops, sorry for this... Signed-off-by: Jiri Pirko <jpirko@redhat.com> > >Oh well, following is probably even more appropriate > >[PATCH net-next-2.6] net: dev_addr_init() fix > >commit f001fde5eadd915f4858d22ed70d7040f48767cf >(net: introduce a list of device addresses dev_addr_list (v6)) >added one regression Vegard Nossum found in its testings. > >dev_addr_init() incorrectly uses sizeof() operator > >Reported-by: Vegard Nossum <vegard.nossum@gmail.com> >Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> >--- >diff --git a/net/core/dev.c b/net/core/dev.c >index 1f38401..65387d9 100644 >--- a/net/core/dev.c >+++ b/net/core/dev.c >@@ -3655,8 +3655,8 @@ static int dev_addr_init(struct net_device *dev) > /* rtnl_mutex must be held here */ > > INIT_LIST_HEAD(&dev->dev_addr_list); >- memset(addr, 0, sizeof(*addr)); >- err = __hw_addr_add(&dev->dev_addr_list, NULL, addr, sizeof(*addr), >+ memset(addr, 0, sizeof(addr)); >+ err = __hw_addr_add(&dev->dev_addr_list, NULL, addr, sizeof(addr), > NETDEV_HW_ADDR_T_LAN); > if (!err) { > /* > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* Eric Dumazet <dada1@cosmosbay.com> wrote: > Eric Dumazet a écrit : > > Vegard Nossum a écrit : > >> 2009/6/7 John Dykstra <john.dykstra1@gmail.com>: > >>> On Sat, 2009-05-30 at 22:23 +0200, Vegard Nossum wrote: > >>>> It seems that loopback's hardware address is never initialized by the > >>>> kernel. So if userspace attempts to read this address before it has > >>>> been set, the kernel will return some uninitialized data (only 6 > >>>> bytes, though). > >>> Thank you for the report, Vegard. > >>> > >>> I've been unable to reproduce the problem you describe, using > >>> 2.6-30-rc8, this test program and a couple of kernel builds for system > >>> load: > >> [...] > >>> ------------------------------------------------------------------ > >>> > >>> Looking at the kernel code, it appears that all bytes of struct > >>> net_device, including the L2 address, are initialized to zeros at > >>> interface creation time. > >>> > >>> Can you spot a difference between your test procedures and mine that > >>> would enable me to reproduce the problem? > >> Hi, > >> > >> I just tried your test program on a linux-next kernel, it works beautifully :-) > >> > >> (I made one change: The stack grows downwards on x86, so I think you > >> should put child_stack + 16386 as the stack to clone()?) > >> > >> As I wrote in reply to Stephen Hemminger, this problem seems to be > >> caused by a particular patch in linux-next: > >> > >> commit f001fde5eadd915f4858d22ed70d7040f48767cf > >> Author: Jiri Pirko <jpirko@redhat.com> > >> Date: Tue May 5 02:48:28 2009 +0000 > >> > >> net: introduce a list of device addresses dev_addr_list (v6) > >> > > > > I believe following patch should fix this problem. > > > > Thank you > > > > [PATCH net-next-2.6] net: loopback device dev->addr_len fix > > > > commit f001fde5eadd915f4858d22ed70d7040f48767cf > > (net: introduce a list of device addresses dev_addr_list (v6)) > > added one regression Vegard Nossum found in its testings. > > > > loopback device doesnt have a hw address, we should set its > > dev->addr_len to 0, not ETH_ALEN. > > > > Reported-by: Vegard Nossum <vegard.nossum@gmail.com> > > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> > > Oh well, following is probably even more appropriate > > [PATCH net-next-2.6] net: dev_addr_init() fix > > commit f001fde5eadd915f4858d22ed70d7040f48767cf > (net: introduce a list of device addresses dev_addr_list (v6)) > added one regression Vegard Nossum found in its testings. > > dev_addr_init() incorrectly uses sizeof() operator > > Reported-by: Vegard Nossum <vegard.nossum@gmail.com> > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> Could you please put the word 'kmemcheck' somewhere into the changelog, to make git-grepping and historic comparisons easier? Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/core/dev.c b/net/core/dev.c index 1f38401..65387d9 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -3655,8 +3655,8 @@ static int dev_addr_init(struct net_device *dev) /* rtnl_mutex must be held here */ INIT_LIST_HEAD(&dev->dev_addr_list); - memset(addr, 0, sizeof(*addr)); - err = __hw_addr_add(&dev->dev_addr_list, NULL, addr, sizeof(*addr), + memset(addr, 0, sizeof(addr)); + err = __hw_addr_add(&dev->dev_addr_list, NULL, addr, sizeof(addr), NETDEV_HW_ADDR_T_LAN); if (!err) { /*