Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

question about privileged mode for labeller #59

Open
rptaylor opened this issue Apr 19, 2024 · 4 comments
Open

question about privileged mode for labeller #59

rptaylor opened this issue Apr 19, 2024 · 4 comments

Comments

@rptaylor
Copy link
Contributor

Description of errors

Labeller daemonset is privileged: https://github.com/ROCm/k8s-device-plugin/blob/master/helm/amd-gpu/templates/labeller.yaml#L66

There is a comment: "#Needed for /dev"

Is read-only access to /dev and /sys sufficient for the labeller to detect the GPU properties?

A pod does not need to be privileged to use a hostPath volume.

What is more, a privileged pod does not need a hostPath to read /dev (it could just nsenter the mount namespace of the host).

More information about the exact requirement of the labeller could help significantly improve the security of the deployment. @y2kenny any insights?

Attach any links, screenshots, or additional evidence you think will be helpful.

No response

@y2kenny
Copy link
Contributor

y2kenny commented Apr 19, 2024

The labeller interact with the driver to get firmware version information:

func GetFirmwareVersions(cardName string) (map[string]uint32, map[string]uint32, error) {

Are you saying that can be done without privileged?

@rptaylor
Copy link
Contributor Author

Maybe. Kubernetes takes care of mounting hostPath volumes, even a completely unprivileged pod can use a hostPath to read in /dev. If writing to the device path is required it would be a different matter. You'd think the device could expose firmware versions with only a read call.

So would it work if

dev, err := os.Open(devPath)
uses a read-only open? @y2kenny

However the other part is that the devices are mode 660.

$ ls -ln /dev/dri
total 0
drwxr-xr-x. 2 0   0      100 Apr  9 11:19 by-path
crw-rw----. 1 0  39 226,   0 Apr  9 11:18 card0
crw-rw----. 1 0  39 226,   1 Apr  9 11:19 card1
crw-rw-rw-. 1 0 998 226, 128 Apr  9 11:19 renderD128

Why not 664 so anyone could read?

@y2kenny
Copy link
Contributor

y2kenny commented Apr 19, 2024

I think 660 is the default for most device. It's not clear to me what read-only mean for a device file since it's more like a pseudo file?

I can potentially just open renderD128 instead but I think the permission is still specific to the system/distro. (I just checked two system I have access to and both render and card are 660.)

@rptaylor
Copy link
Contributor Author

Yes I am also not sure how read/write file permissions map to the interaction with a device using a pseudo file, I guess it depends on how the device API works, in particular if the device file just exposes some readable information or if you have to send a query to it (write) to request driver information.

Is there a disadvantage to read-only access of /dev/dri/renderD128 ? It looks like that is readable to everyone.

If the labeller could run without privileged mode that would be a very nice improvement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants