On Friday, 14 February 2025 12:45:39 CST Darrick J. Wong wrote:
On Fri, Feb 14, 2025 at 12:13:03PM -0600, Elizabeth Figura wrote:
On Friday, 14 February 2025 07:06:20 CST Greg Kroah-Hartman wrote:
On Fri, Feb 14, 2025 at 12:28:00PM +0000, Mike Lothian wrote:
This allows ntsync to be usuable by non-root processes out of the box
Are you sure you need/want that? If so, why? How did existing testing not ever catch this?
Hi, sorry, this is of course my fault.
We do need /dev/ntsync to be openable from user space for it to be useful. I'm not sure what the most "correct" permissions are to have in this case (when we don't specifically need read or write), but I don't think I see a reason not to just set to 666 or 444.
I originally assumed that the right way to do this was not to set the mode on the kernel file but rather through udev; I believe I was using the code for /dev/loop-control or /dev/fuse as an example, which both do that. So I (and others who tested) had just manually set up udev rules for this, with the eventual intent of adding a default rule to systemd like the others. I only recently realized that doing something like this patch is possible and precedented.
I don't know what the best way to address this is, but this is certainly the simplest.
Paranoid defaults in the kernel, and then a udev rule to relax the mode at runtime. You could also have logind scripts to add add per-user allow acls to the device file at user session set up time... or however it is that /dev/sr0 has me on the allow list. I'm not sure how that happens exactly, but it works smoothly.
I get far less complaining about relaxing posture than tightening it (==breaking things) after the fact.
FWIW, it may be worth stressing that this is not a hardware device in any sense, it's a software driver that only lives in a char device (and dedicated module) for the sake of isolating the code. I can't imagine any reason to control access per-user, although my experience may not be enough to grant such imagination.
The only actual risk is a bug in the code itself—which is always possible—but at that point you'd presumably just want to disable it at build time or something similar.