On Sun, Sep 04, 2022 at 06:02:09PM +0200, Alexander Bluhm wrote:
> On Sun, Sep 04, 2022 at 05:42:14PM +0300, Vitaliy Makkoveev wrote:
> > Not for commit, just for collect assertions. Netlock assertions doen't
> > provide panics.
>
> Better use NET_ASSERT_LOCKED_EXCLUSIVE() ?
>
> But maybe nd6 uses a combination of shared netlock and kernel lock.
>
> Is it possible that we sleep somewhere in nd6 input? Then we give
> up kernel lock and shared netlock would not be sufficent. These
> list additions should be safe. But we might sleep during some list
> traversal.
>
I suspect missing netlock in the config path or within timeout handler,
but exclusive netlock assertion makes sense.
Also, we could have "double protection" here, like we have for
`if_list'. I mean we hold both kernel and net locks while we modify
`if_list', but while we do read access, we hold only one of them. I
don't like this, but my diffs for `if_list' were rejected.
Index: sys/net/if.c
===================================================================
RCS file: /cvs/src/sys/net/if.c,v
retrieving revision 1.664
diff -u -p -r1.664 if.c
--- sys/net/if.c 2 Sep 2022 13:12:31 -0000 1.664
+++ sys/net/if.c 4 Sep 2022 16:26:22 -0000
@@ -3148,12 +3148,14 @@ ifpromisc(struct ifnet *ifp, int pswitch
void
ifa_add(struct ifnet *ifp, struct ifaddr *ifa)
{
+ NET_ASSERT_LOCKED_EXCLUSIVE();
TAILQ_INSERT_TAIL(&ifp->if_addrlist, ifa, ifa_list);
}
void
ifa_del(struct ifnet *ifp, struct ifaddr *ifa)
{
+ NET_ASSERT_LOCKED_EXCLUSIVE();
TAILQ_REMOVE(&ifp->if_addrlist, ifa, ifa_list);
}
