Message-ID: <b25a6279-2531-4711-1876-b181fce4acbe@digikod.net>
Date:   Tue, 14 Feb 2023 17:48:05 +0100
MIME-Version: 1.0
User-Agent: 
Subject: Re: [PATCH v5 4/8] LSM: lsm_get_self_attr syscall for LSM self
 attributes
Content-Language: en-US
To:     Paul Moore <paul@paul-moore.com>, Arnd Bergmann <arnd@arndb.de>
Cc:     Casey Schaufler <casey@schaufler-ca.com>,
        casey.schaufler@intel.com, linux-security-module@vger.kernel.org,
        jmorris@namei.org, Kees Cook <keescook@chromium.org>,
        john.johansen@canonical.com,
        Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>,
        stephen.smalley.work@gmail.com, linux-kernel@vger.kernel.org,
        linux-api@vger.kernel.org
References: <20230109180717.58855-1-casey@schaufler-ca.com>
 <20230109180717.58855-5-casey@schaufler-ca.com>
 <f8dca9d2-6e5e-4584-88b3-f71f62988dab@app.fastmail.com>
 <CAHC9VhRRRo9V-q8jT7AbSh1eTPGN7OmxP9hrPz=GTPQa80RjCg@mail.gmail.com>
From:   =?UTF-8?Q?Micka=c3=abl_Sala=c3=bcn?= <mic@digikod.net>
In-Reply-To: <CAHC9VhRRRo9V-q8jT7AbSh1eTPGN7OmxP9hrPz=GTPQa80RjCg@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 12/01/2023 22:39, Paul Moore wrote:
> On Thu, Jan 12, 2023 at 9:40 AM Arnd Bergmann <arnd@arndb.de> wrote:
>> On Mon, Jan 9, 2023, at 19:07, Casey Schaufler wrote:
>>> +/**
>>> + * struct lsm_ctx - LSM context
>>> + * @id: the LSM id number, see LSM_ID_XXX
>>> + * @flags: context specifier and LSM specific flags
>>> + * @ctx_len: the size of @ctx
>>> + * @ctx: the LSM context, a nul terminated string
>>> + *
>>> + * @ctx in a nul terminated string.
>>> + *   (strlen(@ctx) < @ctx_len) is always true.
>>> + *   (strlen(@ctx) == @ctx_len + 1) is not guaranteed.
>>> + */
>>> +struct lsm_ctx {
>>> +     __u32           id;
There is a hole here for 64-bit architectures.

>>> +     __u64           flags;
>>> +     __kernel_size_t ctx_len;

This is an architecture-related size, which makes the struct size 
different according to architectures. We should avoid that.

>>> +     __u8            ctx[];
>>> +};

I suggest packing this struct.

>>
>> I think this should be changed to be the same layout on
>> all architectures regardless of __u64 alignment and
>> sizeof(__kernel_size_t) differences, to avoid the need
>> for compat syscalls and explicit clearing of the
>> internal padding.
>>
>> Maybe just use __u64 fields for all three integers?
> 
> I have no problem with that ... the ctx[] field is variable length
> anyway so keeping it as a __u8 should be fine.
> 

For Landlock, we make sure the UAPI structs don't contain holes,  are 
packed, and have the same size for all architectures. We can check that 
with pahole but for strong guarantee I suggest the same build check as 
for Landlock's build_check_abi(): 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/security/landlock/syscalls.c#n68
We don't need to use 64-bit fields everywhere.