netdev - Re: [RFC PATCH 0/1] Landlock network PoC

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <a1a17348-61f3-9524-c76a-513422ed0332@digikod.net>
Date:   Mon, 3 Jan 2022 12:23:20 +0100
From:   Mickaël Salaün <mic@...ikod.net>
To:     Konstantin Meskhidze <konstantin.meskhidze@...wei.com>
Cc:     linux-security-module@...r.kernel.org, netdev@...r.kernel.org,
        netfilter@...r.kernel.org, yusongping@...wei.com,
        artem.kuzin@...wei.com
Subject: Re: [RFC PATCH 0/1] Landlock network PoC


On 31/12/2021 10:50, Konstantin Meskhidze wrote:
> 12/31/2021 2:26 AM, Mickaël Salaün wrote:

[...]

>>>>> diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
>>>>> index ec72b9262bf3..a335c475965c 100644
>>>>> --- a/security/landlock/ruleset.c
>>>>> +++ b/security/landlock/ruleset.c
>>>>> @@ -27,9 +27,24 @@
>>>>>   static struct landlock_ruleset *create_ruleset(const u32 num_layers)
>>>>>   {
>>>>>       struct landlock_ruleset *new_ruleset;
>>>>> +    u16 row, col, rules_types_num;
>>>>> +
>>>>> +    new_ruleset = kzalloc(sizeof *new_ruleset +
>>>>> +                  sizeof *(new_ruleset->access_masks),
>>>>
>>>> sizeof(access_masks) is 0.
>>>
>>> Actually sizeof *(new_ruleset->access_masks) is 8.
>>> It's a 64 bit pointer to u16 array[]. I checked this
>>> 2D FAM array implementation in a standalone test.
>>
>> Yes, this gives the size of the pointed element, but I wanted to point 
>> out that access_masks doesn't have a size (actually a sizeof() call on 
>> it would failed). This kzalloc() only allocates one element in the 
>> array. What happen when there is more than one layer?
> 
> Here kzalloc() only allocates a pointer to the array;
> The whole array is allocated here:
> 
> rules_types_num = LANDLOCK_RULE_TYPE_NUM;
>      /* Initializes access_mask array for multiple rule types.
>       * Double array semantic is used convenience:
>       * access_mask[rule_type][num_layer].
>       */
>      for (row = 0; row < rules_types_num; row++) {
>          new_ruleset->access_masks[row] = kzalloc(sizeof
>                      *(new_ruleset->access_masks[row]),
>                      GFP_KERNEL_ACCOUNT);
>          for (col = 0; col < num_layers; col++)
>              new_ruleset->access_masks[row][col] = 0;
> 
> If it's needed more the one layer, the code above supports creating
> array of LANDLOCK_RULE_TYPE_NUM*num_layer size (see create_ruleset() 
> function)

Indeed, this should work, but using a 1D array is less complex and 
enables to easily allocate the whole struct+array on contiguous memory.

[...]

>>
>> BTW, you should test with the latest kernel (i.e. latest Linus's tag).
>>
> I thought it was not even important what kernel version to use.
> So I started with the first one with Landlock support. Anyway in future
> it would be easy to rebase landlock network branch or cherry-pick all 
> necessary commits to the latest Linus's tag.

For this patch it should be straightforward because the updated part 
didn't change, but it may not always be the case. Anyway, we always work 
on up-to-date code.