search

ROCm / ROC-smi

ROC System Management Interface
https://github.com/RadeonOpenCompute/ROC-smi/blob/master/README.md
179 stars 55 forks source link

What permissions are required to write to sysfs without sudo? #69

Closed BryantLam closed 5 years ago

BryantLam commented 5 years ago

Today, the rocm-smi script checks to see if you're setting certain values and will relaunch itself as sudo.

Questions

  1. Isn't it sufficient to just let the script error out when users try to write sysfs without the correct permissions? It's not a big deal for me to modify the script to omit the call to relaunchAsSudo, but the call itself seems unnecessary.
  2. Is there a full list of the sysfs values so I can figure out which ones are the most useful to create udev rules for? I think they're all captured in valuePaths. I would like to allow my users to set some of these values (sclk, mclk) that are safer (for some definition) than other values (slevel, mlevel, overdrive).
kentrussell commented 5 years ago

The files are all kept in the valuePaths thankfully, so that makes it easy to refer to. The big thing is that for a standard Linux distribution, the sysfs files are all owned as root:root, so the users would need to be part of the root group or be a root user to change the files (e.g. setting clock speeds, etc). This goes for all of the sysfs files (drmprefix, hwmonprefix, debugprefix, kfdprefix, moduleprefix), they all have root:root ownership.

The only change that can really be made would be to check if the current user has root group access, and relaunch as sudo if they don't, instead of checking if the current user is root. All of the sysfs files require root permissions to modify, so the relaunching as sudo due to requiring those permissions to write to these files is required. This also enables the use of the SMI script in automation, where the appropriate permissions are given to the right users, then no prompts are required.

A lack of root access means that they can't do the action at all (with or without the SMI), so they'd need root permissions to do what they wanted to do. So since they'd need the permissions to do the action, we just relaunch and assume that they have it, since there are no other ways to write to a file with root

BryantLam commented 5 years ago

I understand. My intent was to ask whether it was sufficient to not relaunch as sudo and simply let the write fail due to permission denied. It conveys the same information to the user, but allows the script to also work for operations that don't need sudo. In any case, this issue is somewhat moot now that the script supports reading values without root permissions (which wasn't the case when this issue was initially opened).

Also, given your https://github.com/RadeonOpenCompute/ROC-smi/issues/70#issuecomment-517308302, this approach isn't likely to work anyway.

kentrussell commented 5 years ago

Sounds good. The hard part is that the sysfs/debugfs requirements are all set by the kernel. But if you have an issue where you need to read a value, and it seems to require sudo, raise a bug report so that we can see if we can expose it via sysfs. It's what we did for firmware version, and power consumption, so there is a precedent for creating sysfs files when exclusively read. Writing will always need sudo sadly, so any --set commands will require sudo, or require that the user has root privileges.

Theoretically we could add a --no-sudo flag that ensures that we don't force a reload into sudo, but I'll wait to see if anyone asks for it first :)