On Sat, Jan 03, 2026 at 02:34:38PM +0100, Alexander Bluhm wrote:
> On Sat, Jan 03, 2026 at 06:44:33AM +0300, Vitaliy Makkoveev wrote:
> > On Sat, Jan 03, 2026 at 06:29:19AM +0300, Vitaliy Makkoveev wrote:
> > > I propose to take netlock status instead of `cmd' handrolling. This
> > > relocking is XXX in any case.
> > >
> > > Compile test only.
> > >
> >
> > Please drop previous. We can't know do we hold the shared netlock or
> > not, so handle SIOC*MEDIA and SIOCGIFSFFPAGE cases manually.
>
> These locking dances are ugly and difficult to maintain. When we
> change ioctl(2) locking in the future, drivers may suddenly fail.
> Like in this case. And we do not have hardware for systematic
> testing.
>
Unfortunately, we have some drivers with netlock dances in if_ioctl
path. This is the attempt to enforce the lock between driver's lock and
the netlock, but this hack breaks loops like this:
NET_LOCK();
TAILQ_FOREACH(ifp, &ifnetlist, if_list) {
(*ifp->if_ioctl)(ifp, ...);
}
NET_UNLOCK();
Also, now we have shared netlock, also some cases are called with only
kernel lock held, etc, etc, ...
I believe, we need to introduce optional (*if_lock)() handler, this can
help to avoid relocking at driver side, because the loop above could be
reworked like:
NET_LOCK();
while ((ifp = ifnetlist_iterator(ifp, ...))) {
if (ifp->if_lock) {
NET_UNLOCK();
(*ifp->if_lock)(ifp);
}
(*ifp->if_ioctl)(ifp, ...);
if (ifp->if_lock) {
(*ifp->if_unlock)(ifp);
NET_UNLOCK();
}
}
NET_UNLOCK();
The ifioctl() path is mostly not the problem, the problem lays in the
stack, where we have mix between exclusive and shared netlock and, for
example, socket lock. We can't release netlock here :( Quick look
exposes, we have not so many of them.
Please note, some drivers do copyin() within (*if_ioctl)() handlers, so
we can't use shared netlock + driver lock like we do for inet sockets.
Also, pagefault handler releases exclusive netlock.
If you have interest, I can start with ifnetlist iterator.
