Re: [PATCH v5 15/15] samples/landlock: adds network demo

[Konstantin Meskhidze <konstantin.meskhidze@huawei.com>]

5/19/2022 6:09 PM, Mickaël Salaün пишет:
> 
> 
> On 19/05/2022 15:33, Konstantin Meskhidze wrote:
>>
>>
>> 5/17/2022 12:19 PM, Mickaël Salaün пишет:
>>>
>>>
>>> On 16/05/2022 17:20, Konstantin Meskhidze wrote:
>>>> This commit adds network demo. It's possible to
>>>> allow a sandoxer to bind/connect to a list of
>>>> particular ports restricting networks actions to
>>>> the rest of ports.
>>>>
>>>> Signed-off-by: Konstantin Meskhidze <konstantin.meskhidze@huawei.com>
>>>> ---
>>>>
>>>> Changes since v4:
>>>> * Adds ENV_TCP_BIND_NAME "LL_TCP_BIND" and
>>>> ENV_TCP_CONNECT_NAME "LL_TCP_CONNECT" variables
>>>> to insert TCP ports.
>>>> * Renames populate_ruleset() to populate_ruleset_fs().
>>>> * Adds populate_ruleset_net() and parse_port_num() helpers.
>>>> * Refactoring main() to support network sandboxing.
>>>>
>>>> ---
> 
> [...]
> 
>>>>       if (ruleset_fd < 0) {
>>>>           perror("Failed to create a ruleset");
>>>>           return 1;
>>>>       }
>>>> -    if (populate_ruleset(ENV_FS_RO_NAME, ruleset_fd, access_fs_ro)) {
>>>> +    if (populate_ruleset_fs(ENV_FS_RO_NAME, ruleset_fd, access_fs_ro))
>>>>           goto err_close_ruleset;
>>>> -    }
>>>
>>> Why? I know that checkpatch.pl prints a warning for that but I 
>>> delibirately chooe to use curly braces even for "if" statements with 
>>> one line because it is safer. This code may be copied/pasted and I'd 
>>> like others to avoid introducing goto-fail-like issues.
>>>
>>
>>   It was done just to reduce the number of checkpatch.pl warnings.
>>   If you want it to be formated in your way I will fix it.
> 
> Yes please, checkpatch.pl helps to mantain kernel code but this is a 
> user space code and I prefer to follow safe practices for this kind of 
> checks.
> 
  Ok. I will keep you code here. Thanks.
> [...]
> 
>>>> diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
>>>> index 916b30b31c06..e1ff40f238a6 100644
>>>> --- a/security/landlock/ruleset.h
>>>> +++ b/security/landlock/ruleset.h
>>>> @@ -19,7 +19,7 @@
>>>>   #include "limits.h"
>>>>   #include "object.h"
>>>>
>>>> -typedef u16 access_mask_t;
>>>> +typedef u32 access_mask_t;
>>>
>>> What‽
>>
>>    You are right. I will move this changes to another commit, related 
>> the kernel updates. I might have forgotten to rebase this change and 
>> left it in sandboxer patch. Thank you..
> 
> Indeed. Please check that every commit build (without warning) and that 
> the related tests are OK.

   Ok. I will. Thanks.
> 
> 
>>>
>>>
>>>>
>>>>   /* Makes sure all filesystem access rights can be stored. */
>>>>   static_assert(BITS_PER_TYPE(access_mask_t) >= 
>>>> LANDLOCK_NUM_ACCESS_FS);
>>>> @@ -157,7 +157,7 @@ struct landlock_ruleset {
>>>>                * layers are set once and never changed for the
>>>>                * lifetime of the ruleset.
>>>>                */
>>>> -            u32 access_masks[];
>>>> +            access_mask_t access_masks[];
>>>>           };
>>>>       };
>>>>   };
>>>> -- 
>>>> 2.25.1
>>>>
>>> .
> .