All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kent Gibson <warthog618@gmail.com>
To: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Cc: Bartosz Golaszewski <brgl@kernel.org>,
	Linus Walleij <linusw@kernel.org>,
	linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] gpio: cdev: check if uAPI v2 config attributes are correctly zeroed
Date: Wed, 20 May 2026 19:28:06 +0800	[thread overview]
Message-ID: <20260520112806.GA69947@rigel> (raw)
In-Reply-To: <20260520-gpio-cdev-attr-padding-check-v2-1-0010daf8059f@oss.qualcomm.com>

On Wed, May 20, 2026 at 09:42:10AM +0200, Bartosz Golaszewski wrote:
> We check the padding of other uAPI v2 structures but not that of line
> config attributes. For used attributes: check if their padding is
> zeroed, for unused: check if the entire structure is zeroed.
> 
> Fixes: 3c0d9c635ae2 ("gpiolib: cdev: support GPIO_V2_GET_LINE_IOCTL and GPIO_V2_LINE_GET_VALUES_IOCTL")
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
> ---
> Changes in v2:
> - Make checking even stricter: check if padding is zeroed for used
>   attributes, for unused ones: check if the entire struct is zeroed
> - Link to v1: https://patch.msgid.link/20260519-gpio-cdev-attr-padding-check-v1-1-a0c6d4a698bf@oss.qualcomm.com
> ---
>  drivers/gpio/gpiolib-cdev.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
> index f36b7c06996d70b2286edbd181899e4c572b9086..edbcc86e4b26f88036ed12c13055bb2c371fb6a3 100644
> --- a/drivers/gpio/gpiolib-cdev.c
> +++ b/drivers/gpio/gpiolib-cdev.c
> @@ -1194,6 +1194,15 @@ static int gpio_v2_line_config_validate(struct gpio_v2_line_config *lc,
>  	if (!mem_is_zero(lc->padding, sizeof(lc->padding)))
>  		return -EINVAL;
>  
> +	for (i = 0; i < lc->num_attrs; i++) {
> +		if (lc->attrs[i].attr.padding != 0)
> +			return -EINVAL;
> +	}
> +

That works for me - the explict test against 0 makes the intent as clear
as a mem_is_zero() call. For me anyway.


> +	if (!mem_is_zero(&lc->attrs[i],
> +			 (GPIO_V2_LINE_NUM_ATTRS_MAX - lc->num_attrs) * sizeof(*lc->attrs)))
> +		return -EINVAL;
> +

Dislike the use of i here.
lc->num_attrs would remove the implicit dependency on the preceeding loop.

I'm also uncomfortable with the lc->num_attrs == GPIO_V2_LINE_NUM_ATTRS_MAX
case as the pointer is out of range.  The size is 0 so it probably wont get
dereferenced, but just passing it around makes my skin crawl.
Perhaps put the size in a var and only call mem_is_zero() if size > 0?

Cheers,
Kent.

      reply	other threads:[~2026-05-20 11:28 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-20  7:42 [PATCH v2] gpio: cdev: check if uAPI v2 config attributes are correctly zeroed Bartosz Golaszewski
2026-05-20 11:28 ` Kent Gibson [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260520112806.GA69947@rigel \
    --to=warthog618@gmail.com \
    --cc=bartosz.golaszewski@oss.qualcomm.com \
    --cc=brgl@kernel.org \
    --cc=linusw@kernel.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.