-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/75099/#review226716
-----------------------------------------------------------




src/slave/containerizer/device_manager/device_manager.cpp
Lines 177-197 (patched)
<https://reviews.apache.org/r/75099/#comment314995>

    Ok so this assumes that we've normalized the lists, i.e.
    
    1. wildcard allow entries must not be adding permissions to non-wildcard 
allow entries:
    
    `b 1:* m`
    `b 1:2 rw`
    is not allowed because the wildcard is granting m access, this must be 
normalized to:
    
    `b 1:* m`
    `b 1:2 rwm`
    
    2. Ditto for deny entries.
    
    It would be good to codify this more clearly, e.g.:
    
    1. CgroupDeviceAccess is only constructable in a normalized way (e.g. 
Try<CgroupDeviceAccess> CgroupDeviceAccess::create(...) function that rejects 
invalid.
    
    2. Adding a CHECK(normalized()) inside this function before we run this 
logic. And there's a normalize() function that can be called on it as well by 
whoever is editing the struct.


- Benjamin Mahler


On July 24, 2024, 9:46 p.m., Jason Zhou wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/75099/
> -----------------------------------------------------------
> 
> (Updated July 24, 2024, 9:46 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Currently, we can check if a device access would be granted by directly
> checking the allow and deny lists inside the device manager state for a
> cgroup. This patch adds a helper function to help the user inspect
> whether a device entry would be granted access without needing to know
> the implementation of the ebpf programs.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/device_manager/device_manager.hpp 
> 7c8523d8bdddb8e95c47e1812b48520296680ad6 
>   src/slave/containerizer/device_manager/device_manager.cpp 
> 4c9b86393f0809e08d79b6354940826bd56116f2 
>   src/tests/device_manager_tests.cpp 54d464e97c8fd179128239a6757b16dfa0147c54 
> 
> 
> Diff: https://reviews.apache.org/r/75099/diff/4/
> 
> 
> Testing
> -------
> 
> Added tests for DeviceManager::CgroupDeviceAccess::is_access_granted. Tests 
> passed.
> 
> 
> Thanks,
> 
> Jason Zhou
> 
>

Reply via email to