netdev - Re: [PATCH v7 05/18] landlock: refactor helper functions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Mon, 12 Sep 2022 19:18:00 +0200
From:   Mickaël Salaün <mic@...ikod.net>
To:     "Konstantin Meskhidze (A)" <konstantin.meskhidze@...wei.com>
Cc:     willemdebruijn.kernel@...il.com, gnoack3000@...il.com,
        linux-security-module@...r.kernel.org, netdev@...r.kernel.org,
        netfilter-devel@...r.kernel.org, anton.sirazetdinov@...wei.com
Subject: Re: [PATCH v7 05/18] landlock: refactor helper functions


On 10/09/2022 19:20, Konstantin Meskhidze (A) wrote:
> 
> 
> 9/6/2022 11:07 AM, Mickaël Salaün пишет:

[...]

>>> diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
>>> index 671a95e2a345..84fcd8eb30d4 100644
>>> --- a/security/landlock/ruleset.c
>>> +++ b/security/landlock/ruleset.c
>>> @@ -574,7 +574,8 @@ landlock_find_rule(const struct landlock_ruleset *const ruleset,
>>>     */
>>
>> You missed another hunk from my patch… Please do a diff with it.
> 
>     Sorry. What did I miss here?

There is at least missing comments, please do a diff with my (rebased) 
changes, you'll see.

I wrote all the changes in my commit messages, please include them in 
the related patches (at the correct version).


>>
>>
>>>    bool unmask_layers(const struct landlock_rule *const rule,
>>>    		   const access_mask_t access_request,
>>> -		   layer_mask_t (*const layer_masks)[LANDLOCK_NUM_ACCESS_FS])
>>> +		   layer_mask_t (*const layer_masks)[],
>>> +		   const size_t masks_array_size)
>>>    {
>>>    	size_t layer_level;
>>>
>>> @@ -606,8 +607,7 @@ bool unmask_layers(const struct landlock_rule *const rule,
>>>    		 * requested access.
>>>    		 */
>>>    		is_empty = true;
>>> -		for_each_set_bit(access_bit, &access_req,
>>> -				 ARRAY_SIZE(*layer_masks)) {
>>> +		for_each_set_bit(access_bit, &access_req, masks_array_size) {
>>>    			if (layer->access & BIT_ULL(access_bit))
>>>    				(*layer_masks)[access_bit] &= ~layer_bit;
>>>    			is_empty = is_empty && !(*layer_masks)[access_bit];
>>> @@ -618,15 +618,36 @@ bool unmask_layers(const struct landlock_rule *const rule,
>>>    	return false;
>>>    }
>>>
>>> -access_mask_t
>>> -init_layer_masks(const struct landlock_ruleset *const domain,
>>> -		 const access_mask_t access_request,
>>> -		 layer_mask_t (*const layer_masks)[LANDLOCK_NUM_ACCESS_FS])
>>> +typedef access_mask_t
>>> +get_access_mask_t(const struct landlock_ruleset *const ruleset,
>>> +		  const u16 layer_level);
>>> +
>>> +/*
>>> + * @layer_masks must contain LANDLOCK_NUM_ACCESS_FS or LANDLOCK_NUM_ACCESS_NET
>>> + * elements according to @key_type.
>>> + */
>>> +access_mask_t init_layer_masks(const struct landlock_ruleset *const domain,
>>> +			       const access_mask_t access_request,
>>> +			       layer_mask_t (*const layer_masks)[],
>>> +			       const enum landlock_key_type key_type)
>>>    {
>>>    	access_mask_t handled_accesses = 0;
>>> -	size_t layer_level;
>>> +	size_t layer_level, num_access;
>>> +	get_access_mask_t *get_access_mask;
>>> +
>>> +	switch (key_type) {
>>> +	case LANDLOCK_KEY_INODE:
>>> +		get_access_mask = landlock_get_fs_access_mask;
>>> +		num_access = LANDLOCK_NUM_ACCESS_FS;
>>> +		break;
>>> +	default:
>>> +		WARN_ON_ONCE(1);
>>> +		return 0;
>>> +	}
>>> +
>>> +	memset(layer_masks, 0,
>>> +	       array_size(sizeof((*layer_masks)[0]), num_access));
>>>
>>> -	memset(layer_masks, 0, sizeof(*layer_masks));
>>>    	/* An empty access request can happen because of O_WRONLY | O_RDWR. */
>>>    	if (!access_request)
>>>    		return 0;
>>> @@ -636,9 +657,8 @@ init_layer_masks(const struct landlock_ruleset *const domain,
>>>    		const unsigned long access_req = access_request;
>>>    		unsigned long access_bit;
>>>
>>> -		for_each_set_bit(access_bit, &access_req,
>>> -				 ARRAY_SIZE(*layer_masks)) {
>>> -			if (landlock_get_fs_access_mask(domain, layer_level) &
>>> +		for_each_set_bit(access_bit, &access_req, num_access) {
>>> +			if (get_access_mask(domain, layer_level) &
>>>    			    BIT_ULL(access_bit)) {
>>>    				(*layer_masks)[access_bit] |=
>>>    					BIT_ULL(layer_level);
>>> diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
>>> index d7d9b987829c..2083855bf42d 100644
>>> --- a/security/landlock/ruleset.h
>>> +++ b/security/landlock/ruleset.h
>>> @@ -238,11 +238,12 @@ landlock_get_fs_access_mask(const struct landlock_ruleset *const ruleset,
>>>
>>>    bool unmask_layers(const struct landlock_rule *const rule,
>>>    		   const access_mask_t access_request,
>>> -		   layer_mask_t (*const layer_masks)[LANDLOCK_NUM_ACCESS_FS]);
>>> +		   layer_mask_t (*const layer_masks)[],
>>> +		   const size_t masks_array_size);
>>>
>>> -access_mask_t
>>> -init_layer_masks(const struct landlock_ruleset *const domain,
>>> -		 const access_mask_t access_request,
>>> -		 layer_mask_t (*const layer_masks)[LANDLOCK_NUM_ACCESS_FS]);
>>> +access_mask_t init_layer_masks(const struct landlock_ruleset *const domain,
>>> +			       const access_mask_t access_request,
>>> +			       layer_mask_t (*const layer_masks)[],
>>> +			       const enum landlock_key_type key_type);
>>>
>>>    #endif /* _SECURITY_LANDLOCK_RULESET_H */
>>> --
>>> 2.25.1
>>>
>> .