Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) client-ip=23.128.96.37;
Message-ID: <b62a458c-dd5e-4e8c-bb2f-f997ae329614@embeddedor.com>
Date:   Sun, 26 Nov 2023 08:48:14 -0600
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 1/2] mux: Turn 'mux' into a flexible array in 'struct
 mux_chip'
Content-Language: en-US
To:     Christophe JAILLET <christophe.jaillet@wanadoo.fr>,
        Peter Rosin <peda@axentia.se>,
        Kees Cook <keescook@chromium.org>,
        "Gustavo A. R. Silva" <gustavoars@kernel.org>
Cc:     linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org,
        linux-hardening@vger.kernel.org
References: <d17bd9b622dbe3f7cb2f18736ef3138a6927f86c.1700986053.git.christophe.jaillet@wanadoo.fr>
From:   "Gustavo A. R. Silva" <gustavo@embeddedor.com>
In-Reply-To: <d17bd9b622dbe3f7cb2f18736ef3138a6927f86c.1700986053.git.christophe.jaillet@wanadoo.fr>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 11/26/23 02:08, Christophe JAILLET wrote:
> The 'mux' array stored in 'struct mux_chip' can be changed into a flexible
> array.
> 
> This saves:
>     - a pointer in the structure
>     - an indirection when accessing the array
>     - some pointer arithmetic when computing and storing the address in
>       'mux'
> 
> It is also now possible to use __counted_by() and struct_size() for
> additional safety.
> 
> The address for the 'priv' memory is computed with mux_chip_priv(). It
> should work as good with a flexible array.
> 
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> The struct_size() goodies only work if sizeof_priv is 0. Adding an
> additional size_add() would make it safe in all cases but would make code
> less readable (IMHO).

Just save struct_size() in another variable, and use size_add() like this:

size_t size = struct_size(..);

.. kzalloc(size_add(size, sizeof_priv), ...);

--
Gustavo

> ---
>   drivers/mux/core.c         | 4 +---
>   include/linux/mux/driver.h | 5 +++--
>   2 files changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/mux/core.c b/drivers/mux/core.c
> index 775816112932..80b2607b083b 100644
> --- a/drivers/mux/core.c
> +++ b/drivers/mux/core.c
> @@ -98,13 +98,11 @@ struct mux_chip *mux_chip_alloc(struct device *dev,
>   	if (WARN_ON(!dev || !controllers))
>   		return ERR_PTR(-EINVAL);
>   
> -	mux_chip = kzalloc(sizeof(*mux_chip) +
> -			   controllers * sizeof(*mux_chip->mux) +
> +	mux_chip = kzalloc(struct_size(mux_chip, mux, controllers) +
>   			   sizeof_priv, GFP_KERNEL);
>   	if (!mux_chip)
>   		return ERR_PTR(-ENOMEM);
>   
> -	mux_chip->mux = (struct mux_control *)(mux_chip + 1);
>   	mux_chip->dev.class = &mux_class;
>   	mux_chip->dev.type = &mux_type;
>   	mux_chip->dev.parent = dev;
> diff --git a/include/linux/mux/driver.h b/include/linux/mux/driver.h
> index 18824064f8c0..c29e9b7fb17b 100644
> --- a/include/linux/mux/driver.h
> +++ b/include/linux/mux/driver.h
> @@ -56,18 +56,19 @@ struct mux_control {
>   /**
>    * struct mux_chip -	Represents a chip holding mux controllers.
>    * @controllers:	Number of mux controllers handled by the chip.
> - * @mux:		Array of mux controllers that are handled.
>    * @dev:		Device structure.
>    * @id:			Used to identify the device internally.
>    * @ops:		Mux controller operations.
> + * @mux:		Array of mux controllers that are handled.
>    */
>   struct mux_chip {
>   	unsigned int controllers;
> -	struct mux_control *mux;
>   	struct device dev;
>   	int id;
>   
>   	const struct mux_control_ops *ops;
> +
> +	struct mux_control mux[] __counted_by(controllers);
>   };
>   
>   #define to_mux_chip(x) container_of((x), struct mux_chip, dev)