----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/75099/#review226720 -----------------------------------------------------------
can you break this apart? * add normalize validator and normalizer functions (possibly 2 patches with their own tests) * use them appropriate in device manager to maintain normalized lists * add function for checking if device access is allowed src/linux/cgroups2.cpp Lines 45 (patched) <https://reviews.apache.org/r/75099/#comment314999> using the device manager within cgroups2 library looks odd, instead it seems like normalized should be a function in this library that takes lists and tells you if they are normalized (and another function gives you back normalized lists) src/linux/cgroups2.cpp Lines 1471-1475 (patched) <https://reviews.apache.org/r/75099/#comment315000> this seems strange.. we should probably just have a function in cgroups2 that tells us if device lists are normalized, rather than having to construct an internal struct from DeviceManager here..? src/slave/containerizer/device_manager/device_manager.hpp Lines 75-77 (patched) <https://reviews.apache.org/r/75099/#comment315001> comment on when this would return an error? - Benjamin Mahler On July 25, 2024, 11:48 p.m., Jason Zhou wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/75099/ > ----------------------------------------------------------- > > (Updated July 25, 2024, 11:48 p.m.) > > > Review request for mesos and Benjamin Mahler. > > > Repository: mesos > > > Description > ------- > > Currently we assume that a device state is normalized before using it > for generating ebpf files. However, we have not been enforcing these > constraints on the device access state. > > We enforce some basic validation on cgroups2::configure on the state > to ensure that we are able to generate a correct ebpf program. > An allow or deny list is 'normalized' iff everything below are true: > 1. No Entry can have no accesses specified > 2. No two entries on the same list can have the same type, major & minor > numbers. > 3. No two entries on the same list can be encompassed by the other > entry. > > This patch adds helpers to check if a device state is normalized, > and will only allow users to create new CgroupDeviceAccess instances > using a helper that checks that the allow and deny lists are normalized. > A new helper function is added to check if an entry would be granted > access, and requires the state to be normalized. > > > Diffs > ----- > > src/linux/cgroups2.hpp accaebdaddc85acdd96b87161ea441c77b025099 > src/linux/cgroups2.cpp cb3c425a46f33f5434f870c03dd7de5be3331ece > src/slave/containerizer/device_manager/device_manager.hpp > 7c8523d8bdddb8e95c47e1812b48520296680ad6 > src/slave/containerizer/device_manager/device_manager.cpp > 4c9b86393f0809e08d79b6354940826bd56116f2 > src/tests/containerizer/cgroups2_tests.cpp > c73045632f1bc102d42852b9095a4fe6e11839bb > src/tests/device_manager_tests.cpp 54d464e97c8fd179128239a6757b16dfa0147c54 > > > Diff: https://reviews.apache.org/r/75099/diff/6/ > > > Testing > ------- > > Added tests for DeviceManager::CgroupDeviceAccess::is_access_granted. Tests > passed. > > > Thanks, > > Jason Zhou > >
