search

NVIDIA / libnvidia-container

NVIDIA container runtime library
Apache License 2.0
816 stars 200 forks source link

Issue in permissions checking in nvcgo/internal/cgroup/ebpf.go ? #227

Open oOraph opened 11 months ago

oOraph commented 11 months ago

I'm probably missing something here. Can someone explain me this code part ? https://github.com/NVIDIA/libnvidia-container/blob/main/src/nvcgo/internal/cgroup/ebpf.go#L139C4-L139C4

R3 & bpfAccess == 0 is not enough is it ?

example: device access permissions: r (bpfAccess == unix.BPF_DEVCG_ACC_READ) device access request: r w _ (R3 = unix.BPF_DEVCG_ACC_READ | unix.BPF_DEVCG_ACC_WRITE)

then R3 & bpfAccess != 0 which makes the device a potential candidate to be accepted for write permissions despite the device access being only read ?

Am I missing something ?

Shouldn't we rather have something like this ?

if (R3 & unix.BPF_DEVCG_ACC_READ != 0) && (bpfAccess & unix.BPF_DEVCG_ACC_READ == 0) goto next
if (R3 & unix.BPF_DEVCG_ACC_WRITE != 0) && (bpfAccess & unix.BPF_DEVCG_ACC_WRITE == 0) goto next
if (R3 & unix.BPF_DEVCG_ACC_MKNOD != 0) && (bpfAccess & unix.BPF_DEVCG_ACC_MKNOD == 0) goto next
oOraph commented 10 months ago

Note that I got confirmation that there is indeed an issue:

https://github.com/containers/crun/issues/1343

the check should rather be R3 & bpfAccess != R3 as it is now the case on main branch here:

https://github.com/containers/crun/blob/main/src/libcrun/ebpf.c#L220

And even so it's still arbitrary anyway because then we check for a match with the device permissions at the time of the bpf program forgery, not the time of the bpf program execution

oOraph commented 10 months ago

https://github.com/NVIDIA/libnvidia-container/pull/229