On 17.12.2019 14:19, Maxime Villard wrote: > Le 17/12/2019 à 12:34, Kamil Rytarowski a écrit : >> On 17.12.2019 09:16, Maxime Villard wrote: >>>> Module Name: htdocs >>>> Committed By: christos >>>> Date: Tue Dec 17 01:03:49 UTC 2019 >>>> >>>> Modified Files: >>>> htdocs/support/security: advisory.html index.html >>>> >>>> Log Message: >>>> new advisory >>>> >>>> >>>> To generate a diff of this commit: >>>> cvs rdiff -u -r1.140 -r1.141 htdocs/support/security/advisory.html >>>> cvs rdiff -u -r1.173 -r1.174 htdocs/support/security/index.html >>>> >>>> Please note that diffs are not public domain; they are subject to the >>>> copyright notices on the relevant files. >>> >>> There is something I don't understand here. Why keep this totally >>> useless >>> misfeature, when we already have many tracing facilities that can do >>> just >>> about the same job without having security issues? >>> >>> The recommendation in the advisory is literally to remove the kernel >>> module from the fs. I don't see what could possibly be the use of such a >>> misfeature as filemon; I would remove it completely from the kernel >>> source tree. >>> >>> Maxime >> >> From: >> http://ftp.netbsd.org/pub/NetBSD/security/advisories/NetBSD-SA2019-006.txt.asc >> >> >> "Additionally the way filemon does filesystem interception is racy >> and can lead to random crashes if the system calls are in use >> while the module is unloaded." >> >> Is this issue fixable? Not speaking for filemon in particular, I find >> this ability to rewrite the syscall table on the fly as a feature. >> Keeping a functional module with this property (even if disabled by >> default) seems useful to me. > > As far as I can tell, there are many races caused by autoloading. > > Typically with a character device, the kmod can get unloaded while an ioctl > is being executed on it. When it comes to syscalls, I haven't looked > closely, but the issue is likely the same. > > You can use tricks to "narrow down" the races; eg in NVMM, I use a global > 'nmachines' variable, which prevents unloading in ~most cases. But I see > no way to fix these races, except using atomic refcounts and mutexes on > the ioctls and syscalls; obviously, this won't scale. > > I find this autoloading stuff to be a misfeature, too. If we want something > on by default, then we should put it in GENERIC; if we want it disabled but > accessible, then we should put it in a kmod that requires a manual modload > from root. > > Putting stuff in kmods AND having the kernel load them automatically serves > no purpose; it just adds bugs, and sometimes creates the wrong feeling that > a driver is disabled while it isn't (like filemon). > > Other than that, I don't really understand your point; you can still do > syscall interception with a custom kmod if you want, regardless of filemon. >
Out of filemon, I am only interested in syscall_hijack (filemon_wrapper_install() + filemon_wrapper_deinstall()). If that can be reliable, I would keep it in src/sys/modules/example. https://nxr.netbsd.org/xref/src/sys/dev/filemon/filemon_wrapper.c#90 I have no personal interest on the rest of filemon. Switching this make(1) feature to at least ptrace(2) should be straightforward. > Maxime