Re: [PATCH v5 03/10] bpf: Define handle_fs and add a new helper bpf_handle_fs_get_mode()

2017-03-01 Thread Mickaël Salaün

On 01/03/2017 10:32, James Morris wrote:
> On Wed, 22 Feb 2017, Mickaël Salaün wrote:
> 
>> Add an eBPF function bpf_handle_fs_get_mode(handle_fs) to get the mode
>> of a an abstract object wrapping either a file, a dentry, a path, or an
>> inode.
>>
>> Changes since v4:
>> * use a file abstraction (handle) to wrap inode, dentry, path and file
>>   structs
> 
> Good to see these abstractions.  As discussed at LPC, we need to ensure 
> that we don't couple the Landlock API too closely with the LSM API, as the 
> former is an ABI exposed to userland -- we don't want to lose the ability 
> to change LSM internally due to breaking Landlock policies.

Right, it is the case now, especially with the Landlock events.

> 
>> @@ -82,6 +87,8 @@ enum bpf_arg_type {
>>  
>>  ARG_PTR_TO_CTX, /* pointer to context */
>>  ARG_ANYTHING,   /* any (initialized) argument is ok */
>> +
>> +ARG_CONST_PTR_TO_HANDLE_FS, /* pointer to an abstract FS struct */
>>  };
> 
> Extraneous whitespace?

It is on purpose, following the same rules as used for this enum.

 Mickaël



signature.asc
Description: OpenPGP digital signature


Re: [PATCH v5 03/10] bpf: Define handle_fs and add a new helper bpf_handle_fs_get_mode()

2017-03-01 Thread Mickaël Salaün

On 01/03/2017 10:32, James Morris wrote:
> On Wed, 22 Feb 2017, Mickaël Salaün wrote:
> 
>> Add an eBPF function bpf_handle_fs_get_mode(handle_fs) to get the mode
>> of a an abstract object wrapping either a file, a dentry, a path, or an
>> inode.
>>
>> Changes since v4:
>> * use a file abstraction (handle) to wrap inode, dentry, path and file
>>   structs
> 
> Good to see these abstractions.  As discussed at LPC, we need to ensure 
> that we don't couple the Landlock API too closely with the LSM API, as the 
> former is an ABI exposed to userland -- we don't want to lose the ability 
> to change LSM internally due to breaking Landlock policies.

Right, it is the case now, especially with the Landlock events.

> 
>> @@ -82,6 +87,8 @@ enum bpf_arg_type {
>>  
>>  ARG_PTR_TO_CTX, /* pointer to context */
>>  ARG_ANYTHING,   /* any (initialized) argument is ok */
>> +
>> +ARG_CONST_PTR_TO_HANDLE_FS, /* pointer to an abstract FS struct */
>>  };
> 
> Extraneous whitespace?

It is on purpose, following the same rules as used for this enum.

 Mickaël



signature.asc
Description: OpenPGP digital signature


Re: [PATCH v5 03/10] bpf: Define handle_fs and add a new helper bpf_handle_fs_get_mode()

2017-03-01 Thread James Morris
On Wed, 22 Feb 2017, Mickaël Salaün wrote:

> Add an eBPF function bpf_handle_fs_get_mode(handle_fs) to get the mode
> of a an abstract object wrapping either a file, a dentry, a path, or an
> inode.
> 
> Changes since v4:
> * use a file abstraction (handle) to wrap inode, dentry, path and file
>   structs

Good to see these abstractions.  As discussed at LPC, we need to ensure 
that we don't couple the Landlock API too closely with the LSM API, as the 
former is an ABI exposed to userland -- we don't want to lose the ability 
to change LSM internally due to breaking Landlock policies.

> @@ -82,6 +87,8 @@ enum bpf_arg_type {
>  
>   ARG_PTR_TO_CTX, /* pointer to context */
>   ARG_ANYTHING,   /* any (initialized) argument is ok */
> +
> + ARG_CONST_PTR_TO_HANDLE_FS, /* pointer to an abstract FS struct */
>  };

Extraneous whitespace?


-- 
James Morris



Re: [PATCH v5 03/10] bpf: Define handle_fs and add a new helper bpf_handle_fs_get_mode()

2017-03-01 Thread James Morris
On Wed, 22 Feb 2017, Mickaël Salaün wrote:

> Add an eBPF function bpf_handle_fs_get_mode(handle_fs) to get the mode
> of a an abstract object wrapping either a file, a dentry, a path, or an
> inode.
> 
> Changes since v4:
> * use a file abstraction (handle) to wrap inode, dentry, path and file
>   structs

Good to see these abstractions.  As discussed at LPC, we need to ensure 
that we don't couple the Landlock API too closely with the LSM API, as the 
former is an ABI exposed to userland -- we don't want to lose the ability 
to change LSM internally due to breaking Landlock policies.

> @@ -82,6 +87,8 @@ enum bpf_arg_type {
>  
>   ARG_PTR_TO_CTX, /* pointer to context */
>   ARG_ANYTHING,   /* any (initialized) argument is ok */
> +
> + ARG_CONST_PTR_TO_HANDLE_FS, /* pointer to an abstract FS struct */
>  };

Extraneous whitespace?


-- 
James Morris