Fix applying SELinux label for MPS

[empovit]

If /sys/fs/selinux is not RW-mounted, SELinux will always appear disabled in a container. Trying to apply the label using
go-selinux library fails with lsetxattr /mps/nvidia.com/gpu/pipe: invalid argument. Executing the command, on the other hand, does not fail and applies the label successfully. MPS server and clients can run, the client processes have type M+C as expected.

If /sys/fs/selinux is not mounted, skip applying the label.

[elezar]

Thanks @empovit. I have some minor comments / suggestions.

[elezar]

Do we want to factor this into a function like:

func setSELinuxContext(dir string) error {
	_, err := os.Stat("/sys/fs/selinux")
	if err != nil && !errors.Is(err, os.ErrNotExist) {
		return fmt.Errorf("error checking if SELinux is enabled: %w", err)
	} else if errors.Is(err, os.ErrNotExist) {
		klog.Info("SELinux disabled, not labeling pipe directory")
		return nil
	}

	klog.Info("SELinux enabled, labeling pipe directory")
	chconCmd := exec.Command("chcon", "-R", "-t", "container_file_t", dir)
	output, err := chconCmd.CombinedOutput()
	if err != nil {
		klog.Errorf("\n%v", string(output))
		return fmt.Errorf("error setting SELinux context: %w", err)
	}

	return nil
}

?

[empovit]

Thanks for the review @elezar! Updated.

[elezar]

Does it make sense to create an issue against go-selinux to call out the behaviour we're seeing? I'm not against using the commands as you propose here, but we do lose some flexibility when compared to the functionality in go-selinux.

[empovit]

I've opened opencontainers/selinux/issues/215, but wondering maybe chcon -R -t container_file_t /mps/nvidia.com/gpu/pipe doesn't translate into selinux.Chcon("/mps/nvidia.com/gpu/pipe", "container_file_t", true). On the other hand, I couldn't find any examples of what a label should look like in func Chcon(fpath string, label string, recurse bool) error. The error message wasn't helpful either.

[elezar]

Thanks. Will you retry your experiments with their proposed changes at some point so that we can try and determine why the call is failing?

[empovit]

Yes, sure thing.

[elezar]

When comparing this to the code that is included in the go-selinux pacakge, it is much simpler. I assume this is sufficient for MOST cases for the time being and we can revisit if we find this check too restrictive.

[elezar]

It would also be good to undestand why we would need RW access to this path (as required by go-selinux) instead of assuming that RO access is sufficient as we do here. Would you also ask this question in a go-selinux issue?

[empovit]

I guess https://danwalsh.livejournal.com/73099.html explains it. Assuming I understood your question :)

[empovit]

I believe checking that the path exists actually means checking whether SELinux is present in the system.

[cdesiniotis]

The go-selinux code aligns with the explanation from https://danwalsh.livejournal.com/73099.html. go-selinux always returns disabled unless a selinuxfs is mounted RW at /sys/fs/selinux.

Simply checking that the file exists aligns with what we do in the driver container and does not require any additional mounts, so I am fine with this approach. https://github.com/NVIDIA/gpu-driver-container/blob/24.07.15/rhel8/nvidia-driver#L491-L497

[elezar]

I think from my side this looks good.

It would be good to just also get @cdesiniotis to give his OK.