[LTP] [PATCH 05/10] Add landlock01 test

[Li Wang]

Andrea Cervesato <andrea.cervesato@suse.com> wrote:


> +static void setup(void)
>>> +{
>>> +       verify_landlock_is_enabled();
>>> +
>>> +       rule_size = sizeof(struct landlock_ruleset_attr);
>>> +
>>> +       rule_small_size = rule_size - 1;
>>>
>>
>> Here minus 1 is not enough, we have to decrease more than 8
>> otherwise it just returns ENOMSG.
>>
>>     landlock01.c:49: TFAIL: Size is too small expected EINVAL: ENOMSG (42)
>>
>>   rule_small_size = rule_size - 9;
>>
>> or, we use a small enough type:
>>
>>   rule_small_size = sizeof(int);
>>
>> my test kernel is 6.9.6-200.fc40.x86_64.
>>
>> I haven't figured out that's why, but maybe inside the
>> landlock_create_ruleset()
>> syscall compare the size unprecisely. Something like invoke the
>> copy_min_struct_from_user.
>>
>> 199         err = copy_min_struct_from_user(&ruleset_attr,
>> sizeof(ruleset_attr),
>> 200
>> offsetofend(typeof(ruleset_attr),
>> 201                                          handled_access_fs),
>> 202                                          attr, size);
>>
>> ...
>>
>> 32 #define offsetofend(TYPE, MEMBER) \
>>  33         (offsetof(TYPE, MEMBER) + sizeof_field(TYPE, MEMBER))
>>
>> This needs more investigation...
>>
>
> I guess I found the reason, kernel commit fff69fb03dde1df introducing a new
> field 'handled_access_net' in the structure, but in the
> landlock_create_ruleset(),
> it still uses the first field 'handled_access_fs' to count min size, so
> that why
> decrease 1 is useless in your test.
>
> I will send out a RFC patch to LKML for that. Thanks!
>
> --- a/security/landlock/syscalls.c
> +++ b/security/landlock/syscalls.c
> @@ -198,7 +198,7 @@ SYSCALL_DEFINE3(landlock_create_ruleset,
>         /* Copies raw user space buffer. */
>         err = copy_min_struct_from_user(&ruleset_attr,
> sizeof(ruleset_attr),
>                                         offsetofend(typeof(ruleset_attr),
> -                                                   handled_access_fs),
> +                                                   handled_access_net),
>                                         attr, size);
>         if (err)
>                 return err;
>
>>
>
> --
> Regards,
> Li Wang
>
> First of all, thanks for the review. Super for the bug in the landlock
> code.
>
No problem, but that's maybe on purpose set only one field for
the structure's minimal size, if so, we have to adjust your testcase
to decrese 9.

Let's see what the author thinks about my proposal patch[1].

[1] https://lists.linux.it/pipermail/ltp/2024-July/039073.html


-- 
Regards,
Li Wang