mount/unmount events to devd

[rozhuk.im-gmail.com]

Summary

Send events to devd on mount new file system or unmount existing.

Test Plan

1. Apply patch, rebuild and install kernell, reboot.
2. run: cat /var/run/devd.pipe
3. mount some fs, unmount some fs

1. mount_msdosfs /dev/ada0p3 /media
2. umount /media

!system=kernel subsystem=vfs type=mount fs=msdosfs path="/media"
!system=kernel subsystem=vfs type=unmount fs=msdosfs path="/media"

[mjg]

What's the motivation for this functionality?

/usr/src/sys/kern/vfs_mount.c

842

This is 1296 bytes on the stack which is a non-starter for the kernel. Also 256 is a rather arbitrary number, should be based on the format string instead.

946

This is done too early - should be moved past unlocking the vnode one line below.

Why the device is not added?

There is a serious problem with the format: valid pathnames can contain ", spaces, tabs, newlines and other nasty chars which interfere. in general devctl, as it is, is not very well suited for this data

Note that in certain cases jailed root can mount filesystems and unprivileged users can do the same. This means that a nasty jailed root can try to feed devctl maliciously formatted data to cause trouble.

Same for the unmount case.

[rozhuk.im-gmail.com]

This is alternate to kqueue EVFILT_FS, but there is no need to keep prev state from getmntinfo() and compare with new to find changes.
Also user can use shell scripts to perform additional actions.

/usr/src/sys/kern/vfs_mount.c

842

Is 256->16 ok?

946

I place core near to EVENTHANDLER_DIRECT_INVOKE(), to not break something.
I dont know where to get device.
What can I do with nasty chars? urlencode?

[mjg]

/usr/src/sys/kern/vfs_mount.c

842

Note the most important problem is that the buffer comes from the stack and it is very big. This should be malloc'ed instead.

256 -> 16 does not address the other problem - sizing is completely detached from how much memory is going to be needed. 256 is more than necessary for the current format string, but may become too small at some point. This is just error prone.

946

EVENTHANDLER code passes the vnode as an argument and it keeps it locked. Whether that's necessary or not is another question.

Neither snprintf nor devctl_notify deal with the vnode thus there is no reason to call them with it locked. In general you don't want to do stuff with a lock held if you can avoid it (sometimes performance and other times correctness reasons).

The biggest problem though is that devctl is just not prepared to handle user-supplied data. I don't have a definitive answer how to remedy this problem, but it's definitely a lot of work and it is highly unlikely to be worth the effort for this particular purpose. You did not provide an actual usecase for this functionality.

[imp]

/usr/src/sys/kern/vfs_mount.c

946

devctl is supposed to filter things properly for the protocols involved.
But you need to use devctl_safe_quote_sb to make that happen, which you'd use in conjunction with making the string for buf, which requires a statbuf, which is arguably better than using a stack variable anyway...

[imp]

/usr/src/sys/kern/vfs_mount.c

1484

should likely do the sb stuff here too

[trasz]

Please consider adding it to devd.conf(5) man page.

[rozhuk.im-gmail.com]

Done in https://reviews.freebsd.org/D25969