Re: Unlock in_ioctl_get(), push kernel lock into in_ioctl_{set,change}_ifaddr()

[Klemens Nanni]

On Wed, Nov 30, 2022 at 06:17:51PM +0000, Klemens Nanni wrote:
> Follow up on how in6_ioctl() does it:  grab the kernel lock in all the
> ioctl specific functions, where needed and not earlier, i.e. exactly where
> the net lock is currently taken/released.
> 
> Like in6_ioctl_get(), in_ioctl_get() simply grabs a net lock protected
> interface address, may check net lock protected interface flags and copies
> out data -- all under the shared net lock.
> 
> in_ioctl_set_ifaddr() and in_ioctl_change_ifaddr() remain kernel locked,
> but at least their sanity check on ioctl data now happens without it.
> 
> Feedback? Objection? OK?

Ping.

diff --git a/sys/netinet/in.c b/sys/netinet/in.c
index fa778ef580f..fcecc3ec36a 100644
--- a/sys/netinet/in.c
+++ b/sys/netinet/in.c
@@ -216,9 +216,7 @@ in_control(struct socket *so, u_long cmd, caddr_t data, 
struct ifnet *ifp)
                break;
 #endif /* MROUTING */
        default:
-               KERNEL_LOCK();
                error = in_ioctl(cmd, data, ifp, privileged);
-               KERNEL_UNLOCK();
                break;
        }
 
@@ -262,6 +260,7 @@ in_ioctl(u_long cmd, caddr_t data, struct ifnet *ifp, int 
privileged)
                        return (error);
        }
 
+       KERNEL_LOCK();
        NET_LOCK();
 
        TAILQ_FOREACH(ifa, &ifp->if_addrlist, ifa_list) {
@@ -348,6 +347,7 @@ in_ioctl(u_long cmd, caddr_t data, struct ifnet *ifp, int 
privileged)
        }
 err:
        NET_UNLOCK();
+       KERNEL_UNLOCK();
        return (error);
 }
 
@@ -372,6 +372,7 @@ in_ioctl_set_ifaddr(u_long cmd, caddr_t data, struct ifnet 
*ifp,
        if (error)
                return (error);
 
+       KERNEL_LOCK();
        NET_LOCK();
 
        TAILQ_FOREACH(ifa, &ifp->if_addrlist, ifa_list) {
@@ -406,6 +407,7 @@ in_ioctl_set_ifaddr(u_long cmd, caddr_t data, struct ifnet 
*ifp,
                if_addrhooks_run(ifp);
 
        NET_UNLOCK();
+       KERNEL_UNLOCK();
        return error;
 }
 
@@ -427,6 +429,7 @@ in_ioctl_change_ifaddr(u_long cmd, caddr_t data, struct 
ifnet *ifp,
                        return (error);
        }
 
+       KERNEL_LOCK();
        NET_LOCK();
 
        TAILQ_FOREACH(ifa, &ifp->if_addrlist, ifa_list) {
@@ -555,9 +558,9 @@ in_ioctl_change_ifaddr(u_long cmd, caddr_t data, struct 
ifnet *ifp,
        }
 
        NET_UNLOCK();
+       KERNEL_UNLOCK();
        return (error);
 }
-
 int
 in_ioctl_get(u_long cmd, caddr_t data, struct ifnet *ifp)
 {