Subject: Re: [RFC v2 08/10] landlock: Handle file system comparisons

From: Mickaël Salaün <[email-masked]>
Date: Sat, 27 Aug 2016 15:45:56 +0200

On 26/08/2016 16:57, Andy Lutomirski wrote:
> On Thu, Aug 25, 2016 at 7:10 AM, Mickaël Salaün <[email-masked]> wrote:
>>
>> On 25/08/2016 13:12, Andy Lutomirski wrote:
>>> On Thu, Aug 25, 2016 at 3:32 AM, Mickaël Salaün <[email-masked]> wrote:
>>>> Add eBPF functions to compare file system access with a Landlock file
>>>> system handle:
>>>> * bpf_landlock_cmp_fs_prop_with_struct_file(prop, map, map_op, file)
>>>>   This function allows to compare the dentry, inode, device or mount
>>>>   point of the currently accessed file, with a reference handle.
>>>> * bpf_landlock_cmp_fs_beneath_with_struct_file(opt, map, map_op, file)
>>>>   This function allows an eBPF program to check if the current accessed
>>>>   file is the same or in the hierarchy of a reference handle.
>>>>
>>>> The goal of file system handle is to abstract kernel objects such as a
>>>> struct file or a struct inode. Userland can create this kind of handle
>>>> thanks to the BPF_MAP_UPDATE_ELEM command. The element is a struct
>>>> landlock_handle containing the handle type (e.g.
>>>> BPF_MAP_HANDLE_TYPE_LANDLOCK_FS_FD) and a file descriptor. This could
>>>> also be any descriptions able to match a struct file or a struct inode
>>>> (e.g. path or glob string).
>>>
>>> This needs Eric's opinion.
>>>
>>> Also, where do all the struct file *'s get stashed?  Are they
>>> preserved in the arraymap?  What prevents reference cycles or absurdly
>>> large numbers of struct files getting pinned?
>>
>> Yes, the struct file are kept in the arraymap and dropped when there is
>> no more reference on them. Currently, the limitations are the maximum
>> number of open file descriptors referring to an arraymap and the maximum
>> number of eBPF Landlock programs loaded in a process
>> (LANDLOCK_PROG_LIST_MAX_PAGES in kernel/seccomp.c).
>>
>> What kind of reference cycles have you in mind?
> 
> Shoving evil things into the arraymaps, e.g. unix sockets with
> SCM_RIGHTS messages pending, eBPF program references, the arraymap fd
> itself, another arraymap fd, etc.

The arraymap of Landlock handles is strongly typed and can check the
kind of FD it get when creating/updating an entry, which is done for the
cgroup type. It may be wise to add another check for FS types as well,
which should be a one-liner. I'll do it for the next round.

>>
>> It probably needs another limit for kernel object references as well.
>> What is the best option here? Add another static limitation or use an
>> existing one?
> 
> Dunno.  If RLIMIT_FILE could be made to work, that would be nice.

The RLIMIT_NOFILE is used for the eBPF map creation, but only the memory
limit is used to store the map entries (struct file pointers). I'll add
a new static limit for the number of FD-based arraymap entries because
it does not reflect the same semantic. The struct files are not usable
as FD, their only purpose is to be able to compare with another file.

 Mickaël