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

[Mickaël Salaün <mic@xxxxxxxxxxx>]

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
> > .