On 14/02/17(Tue) 01:50, Alexander Bluhm wrote:
> Hi,
>
> pfctlinput() is called by icmp_input(), so it has the netlock.
>
> Then it calls tcp_ctlinput, tcp_mtudisc, tcp_output, ip_output, so
> it needs the lock. Instead of calling splsoftnet() we should assert
> the netlock in pfctlinput().
>
> There are some paths that seem to miss the lock.
>
> No lock: [pfctlinput, if_down, trunk_port_destroy, trunk_ioctl]
> No lock: [pfctlinput, if_down, trunk_port_destroy, trunk_port_ifdetach]
> No lock: [pfctlinput, if_down, trunk_port_destroy, trunk_clone_destroy]
> No lock: [pfctlinput, if_down, sppp_lcp_down]
> No lock: [pfctlinput, if_down, pppoe_ioctl]
>
> These must either be fixed by removing the XXXSMP in if_clone_destroy()
> or by adding some lock in if_down(). But I want to see the problem
> as splassert first.
No, trunk(4) needs to be fixed. if_clone_destroy() already brings the
interface down. The ioctl(2) handler will hold the lock soon, now that
the unix socket domain problem is fixed. However if_detach() should be
fixed, that's hard.
> ok?
ok mpi@
> Index: kern/uipc_domain.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/kern/uipc_domain.c,v
> retrieving revision 1.48
> diff -u -p -r1.48 uipc_domain.c
> --- kern/uipc_domain.c 20 Dec 2016 21:15:36 -0000 1.48
> +++ kern/uipc_domain.c 14 Feb 2017 00:35:53 -0000
> @@ -227,15 +227,15 @@ pfctlinput(int cmd, struct sockaddr *sa)
> {
> struct domain *dp;
> struct protosw *pr;
> - int i, s;
> + int i;
> +
> + NET_ASSERT_LOCKED();
>
> - s = splsoftnet();
> for (i = 0; (dp = domains[i]) != NULL; i++) {
> for (pr = dp->dom_protosw; pr < dp->dom_protoswNPROTOSW; pr++)
> if (pr->pr_ctlinput)
> (*pr->pr_ctlinput)(cmd, sa, 0, NULL);
> }
> - splx(s);
> }
>
> void
>
