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

On 06/15, Maciej Żenczykowski wrote:
> On Wed, Jun 15, 2022 at 10:38 AM Alexei Starovoitov
> <alexei.starovoitov@...il.com> wrote:
> >
> > On Wed, Jun 15, 2022 at 9:57 AM Maciej Żenczykowski <maze@...gle.com>  
> wrote:
> > > >
> > > > 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.
> > ...
> > > b8bd3ee1971d1edbc53cf322c149ca0227472e56 this is where we added  
> EFAULT in 5.16
> >
> > There are no such sha-s in the upstream kernel.
> > Sorry we cannot help with debugging of android kernels.

> Yes, sdf@ quoted the wrong sha1, it's a clean cherrypick to an
> internal branch of
> 'bpf: Add cgroup helpers bpf_{get,set}_retval to get/set syscall return  
> value'
> commit b44123b4a3dcad4664d3a0f72c011ffd4c9c4d93.

> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=linux-5.16.y&id=b44123b4a3dcad4664d3a0f72c011ffd4c9c4d93

> Anyway, I think it's unrelated - or at least not the immediate root cause.

> Also there's *no* Android kernels involved here.
> This is the android net tests failing on vanilla 5.18 and passing on  
> 5.18.3.

Yeah, sorry, didn't mean to send those outside :-)

Attached un-android-ified testcase. Passes on bpf-next, trying to see
what happens on vanilla 5.18. Will update once I get more data..

-- 

#!/usr/bin/python2.7
# extracted snippet from AOSP, Apache2 licensed

import ctypes
import ctypes.util
import re
import errno
import os
import platform
import struct
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


def CalcSize(fmt):
   if "A" in fmt:
     fmt = fmt.replace("A", "s")
   # Remove the last digital since it will cause error in python3.
   fmt = (re.split('\d+$', fmt)[0])
   return struct.calcsize(fmt)

def CalcNumElements(fmt):
   prevlen = len(fmt)
   fmt = fmt.replace("S", "")
   numstructs = prevlen - len(fmt)
   size = CalcSize(fmt)
   elements = struct.unpack(fmt, b"\x00" * size)
   return len(elements) + numstructs


def Struct(name, fmt, fieldnames, substructs={}):
   """Function that returns struct classes."""

   class Meta(type):

     def __len__(cls):
       return cls._length

     def __init__(cls, unused_name, unused_bases, namespace):
       # Make the class object have the name that's passed in.
       type.__init__(cls, namespace["_name"], unused_bases, namespace)

   class CStruct(object):
     """Class representing a C-like structure."""

     __metaclass__ = Meta

     # Name of the struct.
     _name = name
     # List of field names.
     _fieldnames = fieldnames
     # Dict mapping field indices to nested struct classes.
     _nested = {}
     # List of string fields that are ASCII strings.
     _asciiz = set()

     _fieldnames = _fieldnames.split(" ")

     # Parse fmt into _format, converting any S format characters to "XXs",
     # where XX is the length of the struct type's packed representation.
     _format = ""
     laststructindex = 0
     for i in range(len(fmt)):
       if fmt[i] == "S":
         # Nested struct. Record the index in our struct it should go into.
         index = CalcNumElements(fmt[:i])
         _nested[index] = substructs[laststructindex]
         laststructindex += 1
         _format += "%ds" % len(_nested[index])
       elif fmt[i] == "A":
         # Null-terminated ASCII string.
         index = CalcNumElements(fmt[:i])
         _asciiz.add(index)
         _format += "s"
       else:
         # Standard struct format character.
         _format += fmt[i]

     _length = CalcSize(_format)

     offset_list = [0]
     last_offset = 0
     for i in range(len(_format)):
       offset = CalcSize(_format[:i])
       if offset > last_offset:
         last_offset = offset
         offset_list.append(offset)

     # A dictionary that maps field names to their offsets in the struct.
     _offsets = dict(list(zip(_fieldnames, offset_list)))

     # Check that the number of field names matches the number of fields.
     numfields = len(struct.unpack(_format, b"\x00" * _length))
     if len(_fieldnames) != numfields:
       raise ValueError("Invalid cstruct: \"%s\" has %d elements, \"%s\"  
has %d."
                        % (fmt, numfields, fieldnames, len(_fieldnames)))

     def _SetValues(self, values):
       # Replace self._values with the given list. We can't do direct  
assignment
       # because of the __setattr__ overload on this class.
       super(CStruct, self).__setattr__("_values", list(values))

     def _Parse(self, data):
       data = data[:self._length]
       values = list(struct.unpack(self._format, data))
       for index, value in enumerate(values):
         if isinstance(value, str) and index in self._nested:
           values[index] = self._nested[index](value)
       self._SetValues(values)

     def __init__(self, tuple_or_bytes=None, **kwargs):
       """Construct an instance of this Struct.

       1. With no args, the whole struct is zero-initialized.
       2. With keyword args, the matching fields are populated; rest are  
zeroed.
       3. With one tuple as the arg, the fields are assigned based on  
position.
       4. With one string arg, the Struct is parsed from bytes.
       """
       if tuple_or_bytes and kwargs:
         raise TypeError(
             "%s: cannot specify both a tuple and keyword args" % self._name)

       if tuple_or_bytes is None:
         # Default construct from null bytes.
         self._Parse("\x00" * len(self))
         # If any keywords were supplied, set those fields.
         for k, v in kwargs.items():
           setattr(self, k, v)
       elif isinstance(tuple_or_bytes, str):
         # Initializing from a string.
         if len(tuple_or_bytes) < self._length:
           raise TypeError("%s requires string of length %d, got %d" %
                           (self._name, self._length, len(tuple_or_bytes)))
         self._Parse(tuple_or_bytes)
       else:
         # Initializing from a tuple.
         if len(tuple_or_bytes) != len(self._fieldnames):
           raise TypeError("%s has exactly %d fieldnames (%d given)" %
                           (self._name, len(self._fieldnames),
                            len(tuple_or_bytes)))
         self._SetValues(tuple_or_bytes)

     def _FieldIndex(self, attr):
       try:
         return self._fieldnames.index(attr)
       except ValueError:
         raise AttributeError("'%s' has no attribute '%s'" %
                              (self._name, attr))

     def __getattr__(self, name):
       return self._values[self._FieldIndex(name)]

     def __setattr__(self, name, value):
       # TODO: check value type against self._format and throw here, or else
       # callers get an unhelpful exception when they call Pack().
       self._values[self._FieldIndex(name)] = value

     def offset(self, name):
       if "." in name:
         raise NotImplementedError("offset() on nested field")
       return self._offsets[name]

     @classmethod
     def __len__(cls):
       return cls._length

     def __ne__(self, other):
       return not self.__eq__(other)

     def __eq__(self, other):
       return (isinstance(other, self.__class__) and
               self._name == other._name and
               self._fieldnames == other._fieldnames and
               self._values == other._values)

     @staticmethod
     def _MaybePackStruct(value):
       if hasattr(value, "__metaclass__"):# and value.__metaclass__ == Meta:
         return value.Pack()
       else:
         return value

     def Pack(self):
       values = [self._MaybePackStruct(v) for v in self._values]
       return struct.pack(self._format, *values)

     def __str__(self):
       def FieldDesc(index, name, value):
         if isinstance(value, str):
           if index in self._asciiz:
             value = value.rstrip("\x00")
           elif any(c not in string.printable for c in value):
             value = value.encode("hex")
         return "%s=%s" % (name, value)

       descriptions = [
           FieldDesc(i, n, v) for i, (n, v) in
           enumerate(zip(self._fieldnames, self._values))]

       return "%s(%s)" % (self._name, ", ".join(descriptions))

     def __repr__(self):
       return str(self)

     def CPointer(self):
       """Returns a C pointer to the serialized structure."""
       buf = ctypes.create_string_buffer(self.Pack())
       # Store the C buffer in the object so it doesn't get garbage  
collected.
       super(CStruct, self).__setattr__("_buffer", buf)
       return ctypes.addressof(self._buffer)

   return CStruct




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

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

def VoidPointer(s):
     return ctypes.cast(s.CPointer(), ctypes.c_void_p)

def MaybeRaiseSocketError(ret):
   if ret < 0:
     errno = ctypes.get_errno()
     raise socket.error(errno, os.strerror(errno))

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


def BpfProgLoad(prog_type, instructions, prog_license=b"GPL"):
   bpf_prog = b"".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(instructions),
                                     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()
     self.assertRaisesRegexp(EnvironmentError, os.strerror(errno.EPERM),  
s.sendto, b"foo", addr)

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

Powered by blists - more mailing lists