Date:   Wed, 15 Jun 2022 09:45:31 -0700
From:   Maciej Żenczykowski <zenczykowski@...il.com>
To:     Linux NetDev <netdev@...r.kernel.org>,
        BPF Mailing List <bpf@...r.kernel.org>,
        Maciej Żenczykowski <maze@...gle.com>,
        Stanislav Fomichev <sdf@...gle.com>,
        Alexei Starovoitov <ast@...nel.org>,
        Martin KaFai Lau <kafai@...com>,
        Sasha Levin <sashal@...nel.org>,
        Carlos Llamas <cmllamas@...gle.com>
Subject: Curious bpf regression in 5.18 already fixed in stable 5.18.3

Are you folks aware that:

'bpf: Move rcu lock management out of BPF_PROG_RUN routines'

fixes a weird regression where sendmsg with an egress tc bpf program
denying it was returning EFAULT instead of EPERM

I've confirmed vanilla 5.18.0 is broken, and all it takes is
cherrypicking that specific stable 5.18.x patch [
710a8989b4b4067903f5b61314eda491667b6ab3 ] to fix behaviour.

This was not a flaky failure... but a 100% reproducible behavioural
breakage/failure in the test case at
https://cs.android.com/android/platform/superproject/+/master:kernel/tests/net/test/bpf_test.py;l=517
(where 5.18 would return EFAULT instead of EPERM)

---

A non standalone but perhaps useful (for reading) simplification of
the test case follows.

I was planning on reporting it, hence why I was trying to trim it down
and have this ready anyway, only to discover it's already been fixed.
But the commit message seems to be unrelated...  some sort of compiler
optimization shenanigans?  Missing test case opportunity?

Note: that I run this on x86_64 UML - that might matter??

#!/usr/bin/python
# extracted snippet from AOSP, Apache2 licensed

import csocket
import cstruct
import ctypes
import errno
import os
import platform
import socket
import unittest

__NR_bpf = {  # pylint: disable=invalid-name
    "aarch64-32bit": 386,
    "aarch64-64bit": 280,
    "armv7l-32bit": 386,
    "armv8l-32bit": 386,
    "armv8l-64bit": 280,
    "i686-32bit": 357,
    "i686-64bit": 321,
    "x86_64-32bit": 357,
    "x86_64-64bit": 321,
}[os.uname()[4] + "-" + platform.architecture()[0]]

LOG_LEVEL = 1
LOG_SIZE = 65536

BPF_PROG_LOAD = 5
BPF_PROG_ATTACH = 8
BPF_PROG_DETACH = 9

BPF_PROG_TYPE_CGROUP_SKB = 8

BPF_CGROUP_INET_EGRESS = 1

BPF_REG_0 = 0

BPF_JMP = 0x05
BPF_K = 0x00
BPF_ALU64 = 0x07
BPF_MOV = 0xb0
BPF_EXIT = 0x90

BpfAttrProgLoad = cstruct.Struct(
    "bpf_attr_prog_load", "=IIQQIIQI", "prog_type insn_cnt insns"
    " license log_level log_size log_buf kern_version")
BpfAttrProgAttach = cstruct.Struct(
    "bpf_attr_prog_attach", "=III", "target_fd attach_bpf_fd attach_type")
BpfInsn = cstruct.Struct("bpf_insn", "=BBhi", "code dst_src_reg off imm")

libc = ctypes.CDLL(ctypes.util.find_library("c"), use_errno=True)


def BpfSyscall(op, attr):
  ret = libc.syscall(__NR_bpf, op, csocket.VoidPointer(attr), len(attr))
  csocket.MaybeRaiseSocketError(ret)
  return ret


def BpfProgLoad(prog_type, instructions, prog_license=b"GPL"):
  bpf_prog = "".join(instructions)
  insn_buff = ctypes.create_string_buffer(bpf_prog)
  gpl_license = ctypes.create_string_buffer(prog_license)
  log_buf = ctypes.create_string_buffer(b"", LOG_SIZE)
  return BpfSyscall(BPF_PROG_LOAD,
                    BpfAttrProgLoad((prog_type, len(insn_buff) / len(BpfInsn),
                                    ctypes.addressof(insn_buff),
                                    ctypes.addressof(gpl_license), LOG_LEVEL,
                                    LOG_SIZE, ctypes.addressof(log_buf), 0)))


# Attach a eBPF filter to a cgroup
def BpfProgAttach(prog_fd, target_fd, prog_type):
  attr = BpfAttrProgAttach((target_fd, prog_fd, prog_type))
  return BpfSyscall(BPF_PROG_ATTACH, attr)


# Detach a eBPF filter from a cgroup
def BpfProgDetach(target_fd, prog_type):
  attr = BpfAttrProgAttach((target_fd, 0, prog_type))
  return BpfSyscall(BPF_PROG_DETACH, attr)


class BpfCgroupEgressTest(unittest.TestCase):
  def setUp(self):
    self._cg_fd = os.open("/sys/fs/cgroup", os.O_DIRECTORY | os.O_RDONLY)
    BpfProgAttach(BpfProgLoad(BPF_PROG_TYPE_CGROUP_SKB, [
        BpfInsn((BPF_ALU64 | BPF_MOV | BPF_K, BPF_REG_0, 0,
0)).Pack(),  # Mov64Imm(REG0, 0)
        BpfInsn((BPF_JMP | BPF_EXIT, 0, 0, 0)).Pack()                    # Exit
    ]), self._cg_fd, BPF_CGROUP_INET_EGRESS)

  def tearDown(self):
    BpfProgDetach(self._cg_fd, BPF_CGROUP_INET_EGRESS)
    os.close(self._cg_fd)

  def testCgroupEgressBlocked(self):
    s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM, 0)
    s.bind(("127.0.0.1", 0))
    addr = s.getsockname()
    # previously:   s.sendto("foo", addr)   would fail with EPERM, but
on 5.18+ it EFAULTs
    self.assertRaisesRegexp(EnvironmentError,
os.strerror(errno.EFAULT), s.sendto, "foo", addr)

if __name__ == "__main__":
  unittest.main()

Powered by blists - more mailing lists