Re: [PATCH v1 1/4] landlock: Fix kernel-doc warning for pointer-to-array parameters

["Günther Noack" <gnoack@google.com>]

On Wed, Mar 04, 2026 at 08:31:24PM +0100, Mickaël Salaün wrote:
> The insert_rule() and create_rule() functions take a
> pointer-to-flexible-array parameter declared as:
> 
>   const struct landlock_layer (*const layers)[]
> 
> The kernel-doc parser cannot handle a qualifier between * and the
> parameter name in this syntax, producing spurious "Invalid param" and
> "not described" warnings.
> 
> Introduce landlock_layer_array_t as a typedef for the flexible array
> type so the parameter can be written as:
> 
>   const landlock_layer_array_t *const layers
> 
> This is the same type but kernel-doc parses it correctly, while
> preserving the pointer-to-array type safety that prevents callers from
> accidentally passing a pointer to a single element.
> 
> Cc: Günther Noack <gnoack@google.com>
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> ---
>  security/landlock/ruleset.c | 4 ++--
>  security/landlock/ruleset.h | 8 ++++++++
>  2 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
> index 419b237de635..a61ced492f41 100644
> --- a/security/landlock/ruleset.c
> +++ b/security/landlock/ruleset.c
> @@ -108,7 +108,7 @@ static bool is_object_pointer(const enum landlock_key_type key_type)
>  
>  static struct landlock_rule *
>  create_rule(const struct landlock_id id,
> -	    const struct landlock_layer (*const layers)[], const u32 num_layers,
> +	    const landlock_layer_array_t *const layers, const u32 num_layers,
>  	    const struct landlock_layer *const new_layer)
>  {
>  	struct landlock_rule *new_rule;
> @@ -205,7 +205,7 @@ static void build_check_ruleset(void)
>   */
>  static int insert_rule(struct landlock_ruleset *const ruleset,
>  		       const struct landlock_id id,
> -		       const struct landlock_layer (*const layers)[],
> +		       const landlock_layer_array_t *const layers,
>  		       const size_t num_layers)
>  {
>  	struct rb_node **walker_node;
> diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
> index 9d6dc632684c..87d52031fb5a 100644
> --- a/security/landlock/ruleset.h
> +++ b/security/landlock/ruleset.h
> @@ -37,6 +37,14 @@ struct landlock_layer {
>  	access_mask_t access;
>  };
>  
> +/*
> + * Flexible array of Landlock layers, used for pointer-to-array function
> + * parameters that reference either a stack-allocated layer array or a rule's
> + * flexible array member (struct landlock_rule.layers).  This typedef avoids
> + * the complex (*const name)[] syntax that the kernel-doc parser cannot handle.
> + */
> +typedef struct landlock_layer landlock_layer_array_t[];
> +
>  /**
>   * union landlock_key - Key of a ruleset's red-black tree
>   */
> -- 
> 2.53.0
> 

Thanks for the reminder on the other thread; I skipped over this one
indeed. I am hesitant about this patch because it seems to be at odds
with the Linux kernel coding style on the use of typedef:

https://www.kernel.org/doc/html/v4.17/process/coding-style.html#typedefs

It says:

    the rule should basically be to NEVER EVER use a typedef unless
    you can clearly match one of those rules.

The rules being:

    (a) totally opaque object whose contents we want to hide
        (I don't think that is the purpose here; the example in
	the style guide is to keep generic code from playing with
	hardware-specific page table entry structures)
    (b) integer types (not applicable)
    (c) when using sparse (not applicable)
    (d) some types identical to C99 types (not applicable)
    (e) types safe for use in userspace (not applicable)

It seems that the easier option might be to drop the "const" between
the pointer and the type, if apparently we are the only ones doing
this?

FWIW, I have put these consts as well to be consistent with Landlock
style, but I am also not convinced that they buy us much;

* In a type like "const u8 *buf", when the type is part of a function
  signature, that is a guarantee to the caller that the function won't
  modify the buffer contents through the pointer.

* However, in a type like "u8 *const buf", the const is not a
  guarantee to the caller, but only a constraint on the function
  implementation that the pointer is not rewired to point elsewhere.
  It is not clear to me that this adds much in implementation safety.

WDYT?

—Günther