[PATCH RFC 0/7] proc: move procfs to what kernfs does

Ok, as mentioned off-list I have an RFC version of getting rid of page
fault induced stalls in procfs when removing entry which plagues
networking devices among others.

There was a hackish very narrow AI solution to a part of this bug class
that was floated off-list. It used a second buffering mechanism on top
of seqfiles which was really nasty. Plus, it didn't fix the whole bug
class, it only eliminated a small portion. If we want to get rid of the
whole page fault issue we need a bigger hammer. So I tried to work
through that over the last couple of weeks. The series in a shape where
it's not totally insane to have someone else look at it. So here it is.

I understand it's big but I thought it's probably nicer to look at the
end result than just making a promise of what it might end up looking
like.

It's not super polished yet and I haven't gotten into sorting out all
the warts as there were some really gnarly bugs in there like the whole
readdir mess that repeated entries or terminated early. I think we could
go a lot further there if we wanted to but I think that blows this out
of proportion and at some point it becomes difficult to grasp the scope.
So I'd rather delay that until I have a sense of how acceptable this all
is.

remove_proc_entry() waits until every call into the entry has returned,
and most of those calls copy to or from userspace with the entry pinned.

A reader or writer can make a page fault in that copy take as long as
it wants, with userfaultfd or by punching holes into a memfd over and
over. And the callers of remove_proc_entry() like to hold locks while
they wait. addrconf holds RTNL when it removes /proc/net/dev_snmp6/<dev>
and xt_hashlimit holds a mutex that all network namespaces share.

Every network device with IPv6 gets a /proc/net/dev_snmp6/<dev> that
everyone can read, and addrconf_ifdown() removes it with RTNL held. So
any local user can keep RTNL for as long as their read stays stalled.
Until then no device can be registered, removed or renamed and no
network namespace can be created or torn down.

Without privileges a FUSE mount in a user namespace whose daemon never
answers is enough to stall the copy. Killing the reader ends it, but
until then whoever removes the file sits in D state.

xt_hashlimit removes the proc file of a table with hashlimit_mutex
held. Every rule insert in every network namespace takes that mutex and
so does network namespace cleanup. cleanup_net() runs on a single
workqueue with pernet_ops_rwsem held, so a stalled reader also stops
every later network namespace teardown and every module load that
registers pernet operations. xt_recent does the same with recent_mutex.

And it's not specific to networking. remove_proc_entry(), proc_remove()
and remove_proc_subtree() have over 200 callers and plenty of them hold
locks. A stalled reader of /proc/fs/ext4/<dev>/options holds up the
unmount of that filesystem with s_umount held, and every sync(2) with
it. The same goes for PCI hot-removal, for freeing IRQ descriptors,
which CPU bring-up waits on, and for unplugging a USB sound card.
syzbot has hit removal under locks before (cf. [1]), and the xt_recent
deadlock fixed in (1) below is another one.

Stalling the kernel in a copy to or from userspace is a well-known
exploitation primitive. It's why unprivileged userfaultfd can't do it
by default anymore since v5.11 (d0d4730ac2e4), and FUSE has been used
the same way.

kernfs doesn't have that problem. The code of a kernfs file only ever
runs with the node pinned and never touches userspace memory, kernfs
copies in and out itself. This moves procfs over to the same scheme.

sysfs has only held its active reference around ->show() and ->store()
since v2.6.23, the same release that made remove_proc_entry() wait for
the entry's callers. A copy to userspace couldn't be stalled for long
back then. userfaultfd only came in v4.3 and FUSE in user namespaces in
v4.18. configfs moved its copies out of its active reference in v5.3
and DRM copies ioctl arguments in and out around the driver's handler
the same way. debugfs still holds its reference across the copies,
ran into a deadlock with wifi drivers because of it and grew an API to
cancel operations instead (cf. [2]).

That touches just about every ->proc_read(), ->proc_write() and
->proc_ioctl() in the tree, so I've split it up into seven series. They
go out as replies to this mail:

(1) proc: fix a few old bugs in /proc handlers (38 patches)

    Bugs in the handlers and in procfs itself that are a lot older than
    the conversion and that reviewing it turned up. Most of them are for
    stable. They come first so they can go there on their own and the
    conversion doesn't have to bother with that crap.

(2) proc: let procfs own the data path (16 patches)

    The procfs side. procfs owns the seq_file of an entry, copies in
    writes, copies out reads and copies ioctl arguments in and out, and
    only calls into an entry with it pinned. proc_create_seq() and
    friends and the plain seq_file users are switched over here too.

(3) proc: convert the arch and platform entries (22 patches)
(4) proc: convert the driver entries (23 patches)
(5) proc: convert the fs, kernel and mm entries (27 patches)
(6) proc: convert the net entries and the kfifo samples (11 patches)

(7) proc: drop the __user methods (5 patches)

    Drops the old methods for every entry that can be removed and gives
    the new ones the names kernfs uses, ->proc_read(), ->proc_write()
    and ->proc_ioctl(). This needs all four conversion series.

The series should all indicate cleanly what dependencies they have so b4
should be able to pull it for review. But I could be wrong.

With all of it on a KASAN and lockdep kernel the removal selftests of
every part pass, the proc, pktgen and allocinfo selftests behave like
without it and the bugs from (1) that reproduce in a VM are gone. Every
patch builds on its own on x86_64 and i386, and with W=1 on the
architectures it touches.

[1]: https://lore.kernel.org/r/000000000000f24f4705b6988f3e@google.com (local)
[2]: https://lore.kernel.org/r/20231109212251.213873-7-johannes@sipsolutions.net (local)

-- 
Christian Brauner [off-list ref]