On Sun, Dec 29, 2024 at 08:58:28PM +0000, Al Viro wrote:
> On Sun, Dec 29, 2024 at 08:09:48AM +0000, Al Viro wrote:
>
> > All of that could be avoided if we augmented debugfs inodes with
> > a couple of pointers - no more stashing crap in ->d_fsdata, etc.
> > And it's really not hard to do. The series below attempts to untangle
> > that mess; it can be found in
> > git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git #work.debugfs
> >
> > It's very lightly tested; review and more testing would be
> > very welcome.
>
> BTW, looking through debugfs-related stuff...
>
> arch/x86/kernel/cpu/resctrl/pseudo_lock.c:pseudo_lock_measure_trigger()
> debugfs_file_get()/debugfs_file_put() pair inside, even though the
> only use of that sucker is ->write() of file_operations fed
> to debugfs_create_file(). In other words, it's going to be
> called only by full_proxy_write(), which already has such a pair
> around that...
> drivers/base/regmap/regmap-debugfs.c:regmap_cache_{only,bypass}_write_file()
> ditto
> drivers/gpu/drm/xlnx/zynqmp_dp.c:zynqmp_dp_{pattern,custom}_write()
> ditto
> drivers/gpu/drm/xlnx/zynqmp_dp.c:zynqmp_dp_{pattern,custom}_read()
> similar, except that it's ->read() rather than ->write()
> drivers/infiniband/hw/hfi1/debugfs.c:hfi1_seq_read()
> ditto, AFAICS. Verifying that is not pleasant (use of ## in
> that forest of macros is seriously grep-hostile), but...
> drivers/infiniband/hw/hfi1/debugfs.c:hfi1_seq_lseek()
> same story for ->llseek()
> drivers/infiniband/hw/hfi1/fault.c:fault_opcodes_{read,write}()
> same story
> drivers/thermal/testing/command.c:tt_command_write()
> same, but debugfs_file_put() is apparently lost.
> Attempt to rmmod that sucker ought to deadlock if there had
> been a call of that...
> sound/soc/sof/ipc4-mtrace.c:sof_ipc4_mtrace_dfs_open()
> ->open() calling debugfs_file_get(), with matching debugfs_file_put()
> only in ->release(). Again, that's debugfs_create_file() fodder -
> with nothing to trigger removal in sight. Fortunately, since had
> that been triggerable from userland, you could get a nice deadlock
> by opening that file and triggering removal...
> sound/soc/sof/sof-client-ipc-flood-test.c:sof_ipc_flood_dfs_open()
> same, except that here removal *is* triggerable. Do rmmod with
> e.g. stdin redirected from that file and you are screwed.
> sound/soc/sof/sof-client-ipc-kernel-injector.c:sof_msg_inject_dfs_open()
> same, complete with deadlock on rmmod...
> sound/soc/sof/sof-client-ipc-msg-injector.c:sof_msg_inject_dfs_open()
> ... and here as well.
>
>
> As far as I can see, there's not a single legitimate caller of
> debugfs_file_{get,put}() outside of fs/debugfs/file.c. Is there any
> reason to keep that stuff non-static, let alone exported?
Nope, not at all!
