On Wed, Nov 30, 2022 at 12:24:32AM +0000, Klemens Nanni wrote:
> On Wed, Nov 30, 2022 at 02:25:46AM +0300, Vitaliy Makkoveev wrote:
> > I like to have current "error =" notation for both mrt6_ioctl()
> > and in6_ioctl() within in6_control().
>
> Alright.
>
> > Also, `data’ passed to in6_ioctl_change_ifaddr() is the local
> > variable, kernel lock could be pushed deep down, just before
> > netlock.
Just for prevent misreading. I meant `data' is the pointer to local data
we got from userland.
>
> Sure.
>
> OK?
ok mvs@
> ---
> Neighbour Discovery information is protected by the net lock, as
> documented in nd6.h struct nd_ifinfo.
>
> ndp(8) is the only SIOCGIFINFO_IN6 and SIOCGNBRINFO_IN6 user in base.
>
> nd6_lookup(), also used in ICMP6 input and IPv6 forwarding, only needs
> the net lock.
>
> diff --git a/sys/netinet6/in6.c b/sys/netinet6/in6.c
> index 44dfa3488ef..a34e3860916 100644
> --- a/sys/netinet6/in6.c
> +++ b/sys/netinet6/in6.c
> @@ -213,9 +213,7 @@ in6_control(struct socket *so, u_long cmd, caddr_t data,
> struct ifnet *ifp)
> break;
> #endif /* MROUTING */
> default:
> - KERNEL_LOCK();
> error = in6_ioctl(cmd, data, ifp, privileged);
> - KERNEL_UNLOCK();
> break;
> }
>
> @@ -296,6 +294,7 @@ in6_ioctl_change_ifaddr(u_long cmd, caddr_t data, struct
> ifnet *ifp)
> return (error);
> }
>
> + KERNEL_LOCK();
> NET_LOCK();
>
> if (sa6 != NULL) {
> @@ -402,6 +401,7 @@ in6_ioctl_change_ifaddr(u_long cmd, caddr_t data, struct
> ifnet *ifp)
>
> err:
> NET_UNLOCK();
> + KERNEL_UNLOCK();
> return (error);
> }
>
> @@ -422,6 +422,7 @@ in6_ioctl_get(u_long cmd, caddr_t data, struct ifnet *ifp)
> return (error);
> }
>
> + KERNEL_LOCK();
> NET_LOCK_SHARED();
>
> if (sa6 != NULL) {
> @@ -517,6 +518,7 @@ in6_ioctl_get(u_long cmd, caddr_t data, struct ifnet *ifp)
>
> err:
> NET_UNLOCK_SHARED();
> + KERNEL_UNLOCK();
> return (error);
> }
>
>
