Merge pull request #2796 from vasiliy-ul/ebpf-fix-device-access-check

ebpf: fix device access check
opencontainers · Feb 10, 2021 · 9f13653 · 9f13653
2 parents 2dbfa87 + efb8552
commit 9f13653
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 5 deletions.
diff --git a/libcontainer/cgroups/ebpf/devicefilter/devicefilter.go b/libcontainer/cgroups/ebpf/devicefilter/devicefilter.go
@@ -127,10 +127,10 @@ func (p *program) appendDevice(dev *devices.Rule) error {
  }
  if hasAccess {
  p.insts = append(p.insts,
- // if (R3 & bpfAccess == 0 /* use R1 as a temp var */) goto next
+ // if (R3 & bpfAccess != R3 /* use R1 as a temp var */) goto next
  asm.Mov.Reg32(asm.R1, asm.R3),
  asm.And.Imm32(asm.R1, bpfAccess),
- asm.JEq.Imm(asm.R1, 0, nextBlockSym),
+ asm.JNE.Reg(asm.R1, asm.R3, nextBlockSym),
  )
  }
  if hasMajor {

diff --git a/libcontainer/cgroups/ebpf/devicefilter/devicefilter_test.go b/libcontainer/cgroups/ebpf/devicefilter/devicefilter_test.go
@@ -121,15 +121,15 @@ block-9:
  50: JNEImm dst: r2 off: -1 imm: 1 <block-10>
  51: Mov32Reg dst: r1 src: r3
  52: And32Imm dst: r1 imm: 1
- 53: JEqImm dst: r1 off: -1 imm: 0 <block-10>
+ 53: JNEReg dst: r1 off: -1 src: r3 <block-10>
  54: Mov32Imm dst: r0 imm: 1
  55: Exit
 block-10:
 // (c, wildcard, wildcard, m, true)
  56: JNEImm dst: r2 off: -1 imm: 2 <block-11>
  57: Mov32Reg dst: r1 src: r3
  58: And32Imm dst: r1 imm: 1
- 59: JEqImm dst: r1 off: -1 imm: 0 <block-11>
+ 59: JNEReg dst: r1 off: -1 src: r3 <block-11>
  60: Mov32Imm dst: r0 imm: 1
  61: Exit
 block-11:

diff --git a/tests/integration/dev.bats b/tests/integration/dev.bats
@@ -59,7 +59,7 @@ function teardown() {
 @test "runc run [device cgroup allow rw char device]" {
  requires root
 
- update_config ' .linux.resources.devices = [{"allow": false, "access": "rwm"},{"allow": true, "type": "c", "major": 1, "minor": 11, "access": "rwm"}]
+ update_config ' .linux.resources.devices = [{"allow": false, "access": "rwm"},{"allow": true, "type": "c", "major": 1, "minor": 11, "access": "rw"}]
  | .linux.devices = [{"path": "/dev/kmsg", "type": "c", "major": 1, "minor": 11}]
  | .process.args |= ["sh"]
  | .hostname = "myhostname"'
@@ -75,6 +75,12 @@ function teardown() {
  # test read
  runc exec test_allow_char sh -c 'head -n 1 /dev/kmsg'
  [ "$status" -eq 0 ]
+
+ # test access
+ TEST_NAME="dev_access_test"
+ gcc -static -o "rootfs/bin/${TEST_NAME}" "${TESTDATA}/${TEST_NAME}.c"
+ runc exec test_allow_char sh -c "${TEST_NAME} /dev/kmsg"
+ [ "$status" -eq 0 ]
 }
 
 @test "runc run [device cgroup allow rm block device]" {

diff --git a/tests/integration/testdata/dev_access_test.c b/tests/integration/testdata/dev_access_test.c
@@ -0,0 +1,17 @@
+#include <stdio.h>
+#include <unistd.h>
+
+int main(int argc, char *argv[])
+{
+ const char *dev_name = "/dev/kmsg";
+
+ if (argc > 1)
+ dev_name = argv[1];
+
+ if (access(dev_name, F_OK) < 0) {
+ perror(dev_name);
+ return 1;
+ }
+
+ return 0;
+}