Re: svn commit: r337866 - in head/sys: net netinet netinet6

2018-08-21 Thread Matthew Macy 
On Tue, Aug 21, 2018 at 16:52 Mark Johnston  wrote:

> On Tue, Aug 21, 2018 at 04:00:10PM -0700, Matthew Macy wrote:
> > Yes. See r338162. Thanks.
>
> You missed instances of the same bug in in_mcast.c and in6_mcast.c.



Thanks

>
>
> > On Tue, Aug 21, 2018 at 2:24 PM Gleb Smirnoff 
> wrote:
> > > On Wed, Aug 15, 2018 at 08:23:09PM +, Matt Macy wrote:
> > > M> @@ -3772,8 +3775,11 @@ if_delmulti_locked(struct ifnet *ifp, struct
> > > ifmultiad
> > > M>  ll_ifma->ifma_ifp = NULL;   /* XXX */
> > > M>  if (--ll_ifma->ifma_refcount == 0) {
> > > M>  if (ifp != NULL) {
> > > M> -CK_STAILQ_REMOVE(>if_multiaddrs,
> > > ll_ifma, ifmultiaddr,
> > > M> -ifma_link);
> > > M> +if (ll_ifma->ifma_flags &
> IFMA_F_ENQUEUED)
> > > {
> > > M> +
> > > CK_STAILQ_REMOVE(>if_multiaddrs, ll_ifma, ifmultiaddr,
> > > M> +ifma_link);
> > > M> +ifma->ifma_flags &=
> > > ~IFMA_F_ENQUEUED;
> > > M> +}
> > > M>  }
> > > M>  if_freemulti(ll_ifma);
> > > M>  }
> > >
> > > Coverity suggested there is a cut and paste mistake here, and it is
> > > compilable.
> > > After quick glance I tend to agree. Looks like flag is cleared on wrong
> > > ifma.
> > >
> > > --
> > > Gleb Smirnoff
> > >
>
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r337866 - in head/sys: net netinet netinet6

2018-08-21 Thread Mark Johnston 
On Tue, Aug 21, 2018 at 04:00:10PM -0700, Matthew Macy wrote:
> Yes. See r338162. Thanks.

You missed instances of the same bug in in_mcast.c and in6_mcast.c.

> On Tue, Aug 21, 2018 at 2:24 PM Gleb Smirnoff  wrote:
> > On Wed, Aug 15, 2018 at 08:23:09PM +, Matt Macy wrote:
> > M> @@ -3772,8 +3775,11 @@ if_delmulti_locked(struct ifnet *ifp, struct
> > ifmultiad
> > M>  ll_ifma->ifma_ifp = NULL;   /* XXX */
> > M>  if (--ll_ifma->ifma_refcount == 0) {
> > M>  if (ifp != NULL) {
> > M> -CK_STAILQ_REMOVE(>if_multiaddrs,
> > ll_ifma, ifmultiaddr,
> > M> -ifma_link);
> > M> +if (ll_ifma->ifma_flags & IFMA_F_ENQUEUED)
> > {
> > M> +
> > CK_STAILQ_REMOVE(>if_multiaddrs, ll_ifma, ifmultiaddr,
> > M> +ifma_link);
> > M> +ifma->ifma_flags &=
> > ~IFMA_F_ENQUEUED;
> > M> +}
> > M>  }
> > M>  if_freemulti(ll_ifma);
> > M>  }
> >
> > Coverity suggested there is a cut and paste mistake here, and it is
> > compilable.
> > After quick glance I tend to agree. Looks like flag is cleared on wrong
> > ifma.
> >
> > --
> > Gleb Smirnoff
> >
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r337866 - in head/sys: net netinet netinet6

2018-08-21 Thread Matthew Macy 
Yes. See r338162. Thanks.
-M

On Tue, Aug 21, 2018 at 2:24 PM Gleb Smirnoff  wrote:

> On Wed, Aug 15, 2018 at 08:23:09PM +, Matt Macy wrote:
> M> @@ -3772,8 +3775,11 @@ if_delmulti_locked(struct ifnet *ifp, struct
> ifmultiad
> M>  ll_ifma->ifma_ifp = NULL;   /* XXX */
> M>  if (--ll_ifma->ifma_refcount == 0) {
> M>  if (ifp != NULL) {
> M> -CK_STAILQ_REMOVE(>if_multiaddrs,
> ll_ifma, ifmultiaddr,
> M> -ifma_link);
> M> +if (ll_ifma->ifma_flags & IFMA_F_ENQUEUED)
> {
> M> +
> CK_STAILQ_REMOVE(>if_multiaddrs, ll_ifma, ifmultiaddr,
> M> +ifma_link);
> M> +ifma->ifma_flags &=
> ~IFMA_F_ENQUEUED;
> M> +}
> M>  }
> M>  if_freemulti(ll_ifma);
> M>  }
>
> Coverity suggested there is a cut and paste mistake here, and it is
> compilable.
> After quick glance I tend to agree. Looks like flag is cleared on wrong
> ifma.
>
> --
> Gleb Smirnoff
>
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r337866 - in head/sys: net netinet netinet6

2018-08-21 Thread Gleb Smirnoff 
On Wed, Aug 15, 2018 at 08:23:09PM +, Matt Macy wrote:
M> @@ -3772,8 +3775,11 @@ if_delmulti_locked(struct ifnet *ifp, struct ifmultiad
M>  ll_ifma->ifma_ifp = NULL;   /* XXX */
M>  if (--ll_ifma->ifma_refcount == 0) {
M>  if (ifp != NULL) {
M> -CK_STAILQ_REMOVE(>if_multiaddrs, ll_ifma, 
ifmultiaddr,
M> -ifma_link);
M> +if (ll_ifma->ifma_flags & IFMA_F_ENQUEUED) {
M> +CK_STAILQ_REMOVE(>if_multiaddrs, 
ll_ifma, ifmultiaddr,
M> +ifma_link);
M> +ifma->ifma_flags &= ~IFMA_F_ENQUEUED;
M> +}
M>  }
M>  if_freemulti(ll_ifma);
M>  }

Coverity suggested there is a cut and paste mistake here, and it is compilable.
After quick glance I tend to agree. Looks like flag is cleared on wrong ifma.

-- 
Gleb Smirnoff
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r337866 - in head/sys: net netinet netinet6

2018-08-17 Thread Matthew Macy 
Sorry. I'll take a look.

On Fri, Aug 17, 2018 at 05:30 Andrey V. Elsukov  wrote:

> On 15.08.2018 23:23, Matt Macy wrote:
> > Author: mmacy
> > Date: Wed Aug 15 20:23:08 2018
> > New Revision: 337866
> > URL: https://svnweb.freebsd.org/changeset/base/337866
> >
> > Log:
> >   Fix in6_multi double free
> >
> >   This is actually several different bugs:
> >   - The code is not designed to handle inpcb deletion after interface
> deletion
> > - add reference for inpcb membership
> >   - The multicast address has to be removed from interface lists when
> the refcount
> > goes to zero OR when the interface goes away
> > - decouple list disconnect from refcount (v6 only for now)
> >   - ifmultiaddr can exist past being on interface lists
> > - add flag for tracking whether or not it's enqueued
> >   - deferring freeing moptions makes the incpb cleanup code simpler but
> opens the
> > door wider still to races
> > - call inp_gcmoptions synchronously after dropping the the inpcb lock
> >
> >   Fundamentally multicast needs a rewrite - but keep applying band-aids
> for now.
> >
> >   Tested by: kp
> >   Reported by: novel, kp, lwhsu
> >
> > Modified:
> >   head/sys/net/if.c
> >   head/sys/net/if_var.h
> >   head/sys/netinet/in_mcast.c
> >   head/sys/netinet/in_pcb.c
> >   head/sys/netinet/ip_carp.c
> >   head/sys/netinet6/in6_ifattach.c
> >   head/sys/netinet6/in6_mcast.c
> >   head/sys/netinet6/in6_var.h
> >   head/sys/netinet6/mld6.c
>
> Hi,
>
> After this commit my test machine panics just after boot finishes.
> Reverting this commit helps.
> Machine has two interfaces in failover lagg. One interface is not
> connected.
>
> FreeBSD 12.0-ALPHA2 (GENERIC) #2 r337961M: Fri Aug 17 14:54:48 MSK 2018
>
> # ifconfig
> em0: flags=8843 metric 0 mtu 1500
>
> options=209b
> ether 00:22:4d:6a:5e:b9
> media: Ethernet autoselect
> status: no carrier
> nd6 options=21
> re0: flags=8843 metric 0 mtu 1500
>
> options=8209b
> ether 00:22:4d:6a:5e:b9
> hwaddr 1c:bd:b9:de:0d:7d
> media: Ethernet autoselect (1000baseT )
> status: active
> nd6 options=21
> lagg0: flags=8843 metric 0 mtu 1500
>
> options=209b
> ether 00:22:4d:6a:5e:b9
> inet6 fe80::222:4dff:fe6a:5eb9%lagg0 prefixlen 64 scopeid 0x5
> inet 10.9.8.6 netmask 0xff00 broadcast 10.9.8.255
> laggproto failover lagghash l2,l3,l4
> laggport: em0 flags=1
> laggport: re0 flags=4
> groups: lagg
> media: Ethernet autoselect
> status: active
> nd6 options=21
>
> --
> WBR, Andrey V. Elsukov
>
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r337866 - in head/sys: net netinet netinet6

2018-08-17 Thread Andrey V. Elsukov 
On 15.08.2018 23:23, Matt Macy wrote:
> Author: mmacy
> Date: Wed Aug 15 20:23:08 2018
> New Revision: 337866
> URL: https://svnweb.freebsd.org/changeset/base/337866
> 
> Log:
>   Fix in6_multi double free
>   
>   This is actually several different bugs:
>   - The code is not designed to handle inpcb deletion after interface deletion
> - add reference for inpcb membership
>   - The multicast address has to be removed from interface lists when the 
> refcount
> goes to zero OR when the interface goes away
> - decouple list disconnect from refcount (v6 only for now)
>   - ifmultiaddr can exist past being on interface lists
> - add flag for tracking whether or not it's enqueued
>   - deferring freeing moptions makes the incpb cleanup code simpler but opens 
> the
> door wider still to races
> - call inp_gcmoptions synchronously after dropping the the inpcb lock
>   
>   Fundamentally multicast needs a rewrite - but keep applying band-aids for 
> now.
>   
>   Tested by: kp
>   Reported by: novel, kp, lwhsu
> 
> Modified:
>   head/sys/net/if.c
>   head/sys/net/if_var.h
>   head/sys/netinet/in_mcast.c
>   head/sys/netinet/in_pcb.c
>   head/sys/netinet/ip_carp.c
>   head/sys/netinet6/in6_ifattach.c
>   head/sys/netinet6/in6_mcast.c
>   head/sys/netinet6/in6_var.h
>   head/sys/netinet6/mld6.c

Hi,

After this commit my test machine panics just after boot finishes.
Reverting this commit helps.
Machine has two interfaces in failover lagg. One interface is not connected.

FreeBSD 12.0-ALPHA2 (GENERIC) #2 r337961M: Fri Aug 17 14:54:48 MSK 2018

# ifconfig
em0: flags=8843 metric 0 mtu 1500

options=209b
ether 00:22:4d:6a:5e:b9
media: Ethernet autoselect
status: no carrier
nd6 options=21
re0: flags=8843 metric 0 mtu 1500

options=8209b
ether 00:22:4d:6a:5e:b9
hwaddr 1c:bd:b9:de:0d:7d
media: Ethernet autoselect (1000baseT )
status: active
nd6 options=21
lagg0: flags=8843 metric 0 mtu 1500

options=209b
ether 00:22:4d:6a:5e:b9
inet6 fe80::222:4dff:fe6a:5eb9%lagg0 prefixlen 64 scopeid 0x5
inet 10.9.8.6 netmask 0xff00 broadcast 10.9.8.255
laggproto failover lagghash l2,l3,l4
laggport: em0 flags=1
laggport: re0 flags=4
groups: lagg
media: Ethernet autoselect
status: active
nd6 options=21

-- 
WBR, Andrey V. Elsukov
GNU gdb (GDB) 8.1 [GDB v8.1 for FreeBSD]
Copyright (C) 2018 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later 
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-portbld-freebsd12.0".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
.
Find the GDB manual and other documentation resources online at:
.
For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from /boot/kernel/kernel...Reading symbols from 
/usr/lib/debug//boot/kernel/kernel.debug...done.
done.

Unread portion of the kernel message buffer:
---<>---
Copyright (c) 1992-2018 The FreeBSD Project.
Copyright (c) 1979, 1980, 1983, 1986, 1988, 1989, 1991, 1992, 1993, 1994
The Regents of the University of California. All rights reserved.
FreeBSD is a registered trademark of The FreeBSD Foundation.
FreeBSD 12.0-ALPHA2 #1 r337961M: Fri Aug 17 14:07:50 MSK 2018

butc...@btr-test.yandex.net:/usr/obj/home/devel/freebsd/base/head/amd64.amd64/sys/GENERIC
 amd64
FreeBSD clang version 6.0.1 (tags/RELEASE_601/final 335540) (based on LLVM 
6.0.1)
WARNING: WITNESS option enabled, expect reduced performance.
VT(vga): resolution 640x480
CPU: Intel(R) Core(TM) i5-2400 CPU @ 3.10GHz (3093.05-MHz K8-class CPU)
  Origin="GenuineIntel"  Id=0x206a7  Family=0x6  Model=0x2a  Stepping=7
  
Features=0xbfebfbff
  
Features2=0x1fbae3ff
  AMD Features=0x28100800
  AMD Features2=0x1
  XSAVE Features=0x1
  VT-x: PAT,HLT,MTF,PAUSE,EPT,UG,VPID
  TSC: P-state invariant, performance statistics
real memory  = 25769803776 (24576 MB)
avail memory = 24864264192 (23712 MB)
Event timer "LAPIC" quality 600
ACPI APIC Table: 
FreeBSD/SMP: Multiprocessor System Detected: 4 CPUs
FreeBSD/SMP: 1 package(s) x 4 core(s)
random: unblocking device.
ioapic0  irqs 0-23 on motherboard
Launching APs: 2 1 3
Timecounter "TSC-low" frequency 1546522861 Hz quality 1000
random: entropy device external interface
[ath_hal] loaded
module_register_init: MOD_LOAD (vesa, 0x81101160, 0) error 19
kbd1 at kbdmux0
netmap: loaded module
nexus0
vtvga0:  on motherboard
cryptosoft0:  on motherboard
acpi0:  on motherboard
acpi0: Power Button (fixed)
cpu0:  on acpi0
attimer0:  port 0x40-0x43 irq 0 on acpi0
Timecounter