netdev - Re: [PATCH v8 04/12] landlock: Move unmask_layers() and init_layer

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <f52ca883-599c-0731-e710-f1255810650f@huawei.com>
Date:   Fri, 2 Dec 2022 05:52:57 +0300
From:   "Konstantin Meskhidze (A)" <konstantin.meskhidze@...wei.com>
To:     Mickaël Salaün <mic@...ikod.net>,
        Andy Whitcroft <apw@...onical.com>,
        Joe Perches <joe@...ches.com>,
        Dwaipayan Ray <dwaipayanray1@...il.com>,
        Lukas Bulwahn <lukas.bulwahn@...il.com>
CC:     <willemdebruijn.kernel@...il.com>, <gnoack3000@...il.com>,
        <linux-security-module@...r.kernel.org>, <netdev@...r.kernel.org>,
        <netfilter-devel@...r.kernel.org>, <artem.kuzin@...wei.com>
Subject: Re: [PATCH v8 04/12] landlock: Move unmask_layers() and
 init_layer_masks()



11/28/2022 11:25 PM, Mickaël Salaün пишет:
> 
> On 28/11/2022 04:25, Konstantin Meskhidze (A) wrote:
>> 
>> 
>> 11/17/2022 9:42 PM, Mickaël Salaün пишет:
>>>
>>> On 21/10/2022 17:26, Konstantin Meskhidze wrote:
>>>> This patch moves unmask_layers() and init_layer_masks() helpers
>>>> to ruleset.c to share with landlock network implementation in
>>>
>>> …to share them with the Landlock network implementation in
>>>
>>      Got it.
>>>
>>>> following commits.
>>>>
>>>> Signed-off-by: Konstantin Meskhidze <konstantin.meskhidze@...wei.com>
>>>> ---
> 
> [...]
> 
>>>> diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
>>>> index 608ab356bc3e..50baff4fcbb4 100644
>>>> --- a/security/landlock/ruleset.h
>>>> +++ b/security/landlock/ruleset.h
>>>> @@ -34,6 +34,16 @@ typedef u16 layer_mask_t;
>>>>    /* Makes sure all layers can be checked. */
>>>>    static_assert(BITS_PER_TYPE(layer_mask_t) >= LANDLOCK_MAX_NUM_LAYERS);
>>>>
>>>> +/*
>>>> + * All access rights that are denied by default whether they are handled or not
>>>> + * by a ruleset/layer.  This must be ORed with all ruleset->fs_access_masks[]
>>>> + * entries when we need to get the absolute handled access masks.
>>>> + */
>>>> +/* clang-format off */
>>>> +#define ACCESS_INITIALLY_DENIED ( \
>>>> +	LANDLOCK_ACCESS_FS_REFER)
>>>> +/* clang-format on */
>>>
>>> This ACCESS_INITIALLY_DENIED definition must be moved, not copied. You
>>> can rename ACCESS_INITIALLY_DENIED to ACCESS_FS_INITIALLY_DENIED and
>>> move this hunk before the access_mask_t definition.
>>>
>>     Yep. Will be fixed.
>>>
>>>> +
>>>>    /**
>>>>     * struct landlock_layer - Access rights for a given layer
>>>>     */
>>>> @@ -246,4 +256,14 @@ landlock_get_fs_access_mask(const struct landlock_ruleset *const ruleset,
>>>>    		LANDLOCK_SHIFT_ACCESS_FS) &
>>>>    	       LANDLOCK_MASK_ACCESS_FS;
>>>>    }
>>>> +
>>>> +bool unmask_layers(const struct landlock_rule *const rule,
>>>
>>> All public Landlock helpers must be prefixed with "landlock_"
>> 
>>     Do you mean ones which are shared between fs and net parts?
> 
> All helpers that ends up in the exported ELF symbols, so all implemented
> in the .c files with their signature defined in .h files. The static
> inlined .h helpers don't need to have such prefix if there is no conflict.

   Got it. Thanks.
> 
> 
>>>
>>>> +		   const access_mask_t access_request,
>>>> +		   layer_mask_t (*const layer_masks)[LANDLOCK_NUM_ACCESS_FS]);
>>>> +
>>>> +access_mask_t
>>>> +init_layer_masks(const struct landlock_ruleset *const domain,
>>>> +		 const access_mask_t access_request,
>>>> +		 layer_mask_t (*const layer_masks)[LANDLOCK_NUM_ACCESS_FS]);
>>>
>>> There is a warning generated by checkpatch.pl about this line:
>>>      WARNING: function definition argument 'layer_mask_t' should also have
>>> an identifier name
>>>
>>> I think this is a bug in checkpatch.pl
>>>
>>      I got this warn, but cant get rid of it.
>>      Also think its a bug in checkpatck.pl
> 
> Please ignore it for now. It would be nice to have a checkpatch.pl fix
> though.
> 
   Ok.
>> 
>>> Any though Andy, Joe, Dwaipayan or Lukas?
> .