Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <f8f5ef29-4446-8f19-08a3-b9e080855405@digikod.net>
Date:   Thu, 24 Mar 2022 11:31:32 +0100
MIME-Version: 1.0
User-Agent: 
Subject: Re: [PATCH v1 06/11] landlock: Add support for file reparenting with
 LANDLOCK_ACCESS_FS_REFER
Content-Language: en-US
To:     Paul Moore <paul@paul-moore.com>
Cc:     James Morris <jmorris@namei.org>,
        "Serge E . Hallyn" <serge@hallyn.com>,
        Al Viro <viro@zeniv.linux.org.uk>,
        Jann Horn <jannh@google.com>,
        Kees Cook <keescook@chromium.org>,
        Konstantin Meskhidze <konstantin.meskhidze@huawei.com>,
        Shuah Khan <shuah@kernel.org>, linux-doc@vger.kernel.org,
        linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-security-module@vger.kernel.org
References: <20220221212522.320243-1-mic@digikod.net>
 <20220221212522.320243-7-mic@digikod.net>
 <CAHC9VhSFXN39EuVG5aVK0jtgCOmzM2FSCoVa2Xrs=oJQ4AkWMQ@mail.gmail.com>
 <588e0fec-6a45-db81-e411-ae488b29e533@digikod.net>
From:   =?UTF-8?Q?Micka=c3=abl_Sala=c3=bcn?= <mic@digikod.net>
In-Reply-To: <588e0fec-6a45-db81-e411-ae488b29e533@digikod.net>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk


On 17/03/2022 13:04, Mickaël Salaün wrote:
> 
> On 17/03/2022 02:26, Paul Moore wrote:

[...]

>>> @@ -269,16 +270,188 @@ static inline bool is_nouser_or_private(const 
>>> struct dentry *dentry)
>>>                           
>>> unlikely(IS_PRIVATE(d_backing_inode(dentry))));
>>>   }
>>>
>>> -static int check_access_path(const struct landlock_ruleset *const 
>>> domain,
>>> -               const struct path *const path,
>>> +static inline access_mask_t get_handled_accesses(
>>> +               const struct landlock_ruleset *const domain)
>>> +{
>>> +       access_mask_t access_dom = 0;
>>> +       unsigned long access_bit;
>>
>> Would it be better to declare @access_bit as an access_mask_t type?
>> You're not using any macros like for_each_set_bit() in this function
>> so I believe it should be safe.
> 
> Right, I'll change that.

Well, thinking about it again, access_bit is not an access mask but an 
index in such mask. access_mask_t gives enough space for such index but 
it is definitely not the right semantic. The best type should be size_t, 
but I prefer to stick to unsigned long (used for size_t anyway) for 
consistency with the other access_bit variable types. There is no need 
to use for_each_set_bit() here now but that could change, and I prefer 
to do my best to prevent future issues. ;)
Anyway, I guess the compiler can optimize such code.