refcnt_init starts counting at 1, while the existing code starts at 0. Do the crashes stop because we never fully release all the references and never free it now?
On Sat, 6 Feb 2021, 10:55 Alexander Bluhm, <alexander.bl...@gmx.net> wrote: > Hi, > > When I replace the ++ and -- of ifg_refcnt with an atomic operation, > it fixes this syzkaller panic. > > > https://syzkaller.appspot.com/bug?id=54e16dc5bce6929e14b42e2f1379f1c18f62be43 > > Without the fix "syz-execprog -repeat=0 -procs=8 repro-pfi.syz" > crashes my vmm in a few seconds. With the diff I cannot reproduce > for several minutes. > > ok? > > bluhm > > Index: net/if.c > =================================================================== > RCS file: /cvs/src/sys/net/if.c,v > retrieving revision 1.626 > diff -u -p -r1.626 if.c > --- net/if.c 1 Feb 2021 07:43:33 -0000 1.626 > +++ net/if.c 6 Feb 2021 00:37:50 -0000 > @@ -2601,7 +2601,7 @@ if_creategroup(const char *groupname) > return (NULL); > > strlcpy(ifg->ifg_group, groupname, sizeof(ifg->ifg_group)); > - ifg->ifg_refcnt = 0; > + refcnt_init(&ifg->ifg_refcnt); > ifg->ifg_carp_demoted = 0; > TAILQ_INIT(&ifg->ifg_members); > #if NPF > 0 > @@ -2648,7 +2648,7 @@ if_addgroup(struct ifnet *ifp, const cha > return (ENOMEM); > } > > - ifg->ifg_refcnt++; > + refcnt_take(&ifg->ifg_refcnt); > ifgl->ifgl_group = ifg; > ifgm->ifgm_ifp = ifp; > > @@ -2692,7 +2692,7 @@ if_delgroup(struct ifnet *ifp, const cha > pfi_group_change(groupname); > #endif > > - if (--ifgl->ifgl_group->ifg_refcnt == 0) { > + if (refcnt_rele(&ifgl->ifgl_group->ifg_refcnt)) { > TAILQ_REMOVE(&ifg_head, ifgl->ifgl_group, ifg_next); > #if NPF > 0 > pfi_detach_ifgroup(ifgl->ifgl_group); > Index: net/if_var.h > =================================================================== > RCS file: /cvs/src/sys/net/if_var.h,v > retrieving revision 1.112 > diff -u -p -r1.112 if_var.h > --- net/if_var.h 29 Jul 2020 12:09:31 -0000 1.112 > +++ net/if_var.h 6 Feb 2021 00:38:23 -0000 > @@ -263,7 +263,7 @@ struct ifmaddr { > > struct ifg_group { > char ifg_group[IFNAMSIZ]; > - u_int ifg_refcnt; > + struct refcnt ifg_refcnt; > caddr_t ifg_pf_kif; > int ifg_carp_demoted; > TAILQ_HEAD(, ifg_member) ifg_members; > Index: netinet/ip_carp.c > =================================================================== > RCS file: /cvs/src/sys/netinet/ip_carp.c,v > retrieving revision 1.351 > diff -u -p -r1.351 ip_carp.c > --- netinet/ip_carp.c 21 Jan 2021 13:18:07 -0000 1.351 > +++ netinet/ip_carp.c 6 Feb 2021 00:39:14 -0000 > @@ -789,7 +789,7 @@ carpattach(int n) > struct ifg_group *ifg; > > if ((ifg = if_creategroup("carp")) != NULL) > - ifg->ifg_refcnt++; /* keep around even if empty */ > + refcnt_take(&ifg->ifg_refcnt); /* keep around even if > empty */ > if_clone_attach(&carp_cloner); > carpcounters = counters_alloc(carps_ncounters); > } > >
