[PATCHv3] gpio: virtio: remove one kcalloc

[PATCHv3] gpio: virtio: remove one kcalloc

[Rosen Penev]

A flexible array member can be used to combine allocations.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 v3: add counting field for __counted_by.
 v2: add space in struct
 drivers/gpio/gpio-virtio.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpio/gpio-virtio.c b/drivers/gpio/gpio-virtio.c
index ed6e0e90fa8a..57d0eb532c3c 100644
--- a/drivers/gpio/gpio-virtio.c
+++ b/drivers/gpio/gpio-virtio.c
@@ -52,7 +52,6 @@ struct virtio_gpio {
 	struct virtio_device *vdev;
 	struct mutex lock; /* Protects virtqueue operation */
 	struct gpio_chip gc;
-	struct virtio_gpio_line *lines;
 	struct virtqueue *request_vq;

 	/* irq support */
@@ -60,6 +59,9 @@ struct virtio_gpio {
 	struct mutex irq_lock; /* Protects irq operation */
 	raw_spinlock_t eventq_lock; /* Protects queuing of the buffer */
 	struct vgpio_irq_line *irq_lines;
+
+	u16 ngpio;
+	struct virtio_gpio_line lines[] __counted_by(ngpio);
 };

 static int _virtio_gpio_req(struct virtio_gpio *vgpio, u16 type, u16 gpio,
@@ -541,10 +543,6 @@ static int virtio_gpio_probe(struct virtio_device *vdev)
 	u16 ngpio;
 	int ret, i;

-	vgpio = devm_kzalloc(dev, sizeof(*vgpio), GFP_KERNEL);
-	if (!vgpio)
-		return -ENOMEM;
-
 	/* Read configuration */
 	gpio_names_size =
 		virtio_cread32(vdev, offsetof(struct virtio_gpio_config,
@@ -556,10 +554,12 @@ static int virtio_gpio_probe(struct virtio_device *vdev)
 		return -EINVAL;
 	}

-	vgpio->lines = devm_kcalloc(dev, ngpio, sizeof(*vgpio->lines), GFP_KERNEL);
-	if (!vgpio->lines)
+	vgpio = devm_kzalloc(dev, struct_size(vgpio, lines, ngpio), GFP_KERNEL);
+	if (!vgpio)
 		return -ENOMEM;

+	vgpio->ngpio = ngpio;
+
 	for (i = 0; i < ngpio; i++) {
 		mutex_init(&vgpio->lines[i].lock);
 		init_completion(&vgpio->lines[i].completion);
--
2.53.0

Re: [PATCHv3] gpio: virtio: remove one kcalloc

[Viresh Kumar]

On 12-03-26, 12:37, Rosen Penev wrote:
> A flexible array member can be used to combine allocations.
> 
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  v3: add counting field for __counted_by.
>  v2: add space in struct
>  drivers/gpio/gpio-virtio.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-virtio.c b/drivers/gpio/gpio-virtio.c
> index ed6e0e90fa8a..57d0eb532c3c 100644
> --- a/drivers/gpio/gpio-virtio.c
> +++ b/drivers/gpio/gpio-virtio.c
> @@ -52,7 +52,6 @@ struct virtio_gpio {
>  	struct virtio_device *vdev;
>  	struct mutex lock; /* Protects virtqueue operation */
>  	struct gpio_chip gc;
> -	struct virtio_gpio_line *lines;
>  	struct virtqueue *request_vq;
> 
>  	/* irq support */
> @@ -60,6 +59,9 @@ struct virtio_gpio {
>  	struct mutex irq_lock; /* Protects irq operation */
>  	raw_spinlock_t eventq_lock; /* Protects queuing of the buffer */
>  	struct vgpio_irq_line *irq_lines;
> +
> +	u16 ngpio;
> +	struct virtio_gpio_line lines[] __counted_by(ngpio);
>  };

I wonder if it is worth it anymore. Why combining allocations is better when we
are ending up using more memory ?

-- 
viresh

Re: [PATCHv3] gpio: virtio: remove one kcalloc

[Rosen Penev]

On Thu, Mar 12, 2026 at 11:09 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 12-03-26, 12:37, Rosen Penev wrote:
> > A flexible array member can be used to combine allocations.
> >
> > Signed-off-by: Rosen Penev <rosenp@gmail.com>
> > Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
> > ---
> >  v3: add counting field for __counted_by.
> >  v2: add space in struct
> >  drivers/gpio/gpio-virtio.c | 14 +++++++-------
> >  1 file changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpio/gpio-virtio.c b/drivers/gpio/gpio-virtio.c
> > index ed6e0e90fa8a..57d0eb532c3c 100644
> > --- a/drivers/gpio/gpio-virtio.c
> > +++ b/drivers/gpio/gpio-virtio.c
> > @@ -52,7 +52,6 @@ struct virtio_gpio {
> >       struct virtio_device *vdev;
> >       struct mutex lock; /* Protects virtqueue operation */
> >       struct gpio_chip gc;
> > -     struct virtio_gpio_line *lines;
> >       struct virtqueue *request_vq;
> >
> >       /* irq support */
> > @@ -60,6 +59,9 @@ struct virtio_gpio {
> >       struct mutex irq_lock; /* Protects irq operation */
> >       raw_spinlock_t eventq_lock; /* Protects queuing of the buffer */
> >       struct vgpio_irq_line *irq_lines;
> > +
> > +     u16 ngpio;
> > +     struct virtio_gpio_line lines[] __counted_by(ngpio);
> >  };
>
> I wonder if it is worth it anymore. Why combining allocations is better when we
> are ending up using more memory ?
No idea. That's what maintainers suggested for some unknown reason.

Anyway, I don't care if this gets merged anymore.
>
> --
> viresh

Re: [PATCHv3] gpio: virtio: remove one kcalloc

[Linus Walleij]

On Fri, Mar 13, 2026 at 7:30 AM Rosen Penev <rosenp@gmail.com> wrote:

> > I wonder if it is worth it anymore. Why combining allocations is better when we
> > are ending up using more memory ?

> No idea. That's what maintainers suggested for some unknown reason.

This YouTube seminar by Kees Cook and Gustavo Silva explains
exactly why we want this:
https://www.youtube.com/watch?v=V2kzptQG5_A

As mentioned briefly in my reply to Viresh, it brings some of the same
features that Rust has to the kernel C dialect.

This is done to avoid what we call undefined behaviour.

Yours,
Linus Walleij

Re: [PATCHv3] gpio: virtio: remove one kcalloc

[Linus Walleij]

On Fri, Mar 13, 2026 at 7:09 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:

> I wonder if it is worth it anymore. Why combining allocations is better when we
> are ending up using more memory ?

For the same reason we are starting to use Rust in the kernel, despite
it sometimes will take more memory essentially. __counted_by() enforce
the same type of runtime size checks as Rust do on arrays.

Yours,
Linus Walleij

Re: [PATCHv3] gpio: virtio: remove one kcalloc

[Viresh Kumar]

On 16-03-26, 15:00, Linus Walleij wrote:
> On Fri, Mar 13, 2026 at 7:09 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> 
> > I wonder if it is worth it anymore. Why combining allocations is better when we
> > are ending up using more memory ?
> 
> For the same reason we are starting to use Rust in the kernel, despite
> it sometimes will take more memory essentially. __counted_by() enforce
> the same type of runtime size checks as Rust do on arrays.

Right. I don't have any issue with __counted_by(). It does the right thing for
flexible length arrays. But we don't need a flexible length array here and so my
question.

-- 
viresh

Re: [PATCHv3] gpio: virtio: remove one kcalloc

[Linus Walleij]

On Tue, Mar 17, 2026 at 9:49 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 16-03-26, 15:00, Linus Walleij wrote:
> > On Fri, Mar 13, 2026 at 7:09 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> >
> > > I wonder if it is worth it anymore. Why combining allocations is better when we
> > > are ending up using more memory ?
> >
> > For the same reason we are starting to use Rust in the kernel, despite
> > it sometimes will take more memory essentially. __counted_by() enforce
> > the same type of runtime size checks as Rust do on arrays.
>
> Right. I don't have any issue with __counted_by(). It does the right thing for
> flexible length arrays. But we don't need a flexible length array here and so my
> question.

So why check for something that "can't go wrong".

IIUC it still removes undefined behaviour from the object code.

If someone managed to compromise the kernel using return-oriented programming
they cannot call back into this function to overwrite the memory
beyond where the
array is stored, because the runtime checks will block this.

But Kees & Gustavo can tell if I understand this correctly.

Yours,
Linus Walleij

Re: [PATCHv3] gpio: virtio: remove one kcalloc

[Linus Walleij]

On Thu, Mar 12, 2026 at 8:37 PM Rosen Penev <rosenp@gmail.com> wrote:

> A flexible array member can be used to combine allocations.
>
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

Reviewed-by: Linus Walleij <linusw@kernel.org>

Yours,
Linus Walleij