All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kees Cook <kees@kernel.org>
To: Rosen Penev <rosenp@gmail.com>
Cc: platform-driver-x86@vger.kernel.org,
	"Kenneth Chan" <kenneth.t.chan@gmail.com>,
	"Hans de Goede" <hansg@kernel.org>,
	"Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>,
	"Gustavo A. R. Silva" <gustavoars@kernel.org>,
	"open list" <linux-kernel@vger.kernel.org>,
	"open list:KERNEL HARDENING (not covered by other
	areas):Keyword:b__counted_by(_le|_be)?b"
	<linux-hardening@vger.kernel.org>
Subject: Re: [PATCH] platform/x86: panasonic-laptop: simplify allocation of sinf
Date: Fri, 20 Mar 2026 11:32:37 -0700	[thread overview]
Message-ID: <202603201131.55E516E@keescook> (raw)
In-Reply-To: <20260320004928.29914-1-rosenp@gmail.com>

On Thu, Mar 19, 2026 at 05:49:28PM -0700, Rosen Penev wrote:
> Change to a flexible array member to allocate once instead of twice.
> 
> Allows using __counted_by for extra runtime analysis. Move the counting
> variable assignment to right after allocation as required by
> __counted_by.
> 
> Remove + 1 to allocation. It's already done in the previous line.

Are you sure this is an accidental +1? I see the "num_sifr++" that
happens earlier, but it's not immediately clear why either that or the
+1 in the original allocation are needed. I'd like to understand why
either/both are/aren't needed.

-Kees

> 
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  drivers/platform/x86/panasonic-laptop.c | 17 ++++-------------
>  1 file changed, 4 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/platform/x86/panasonic-laptop.c b/drivers/platform/x86/panasonic-laptop.c
> index d923ddaa4849..4d663fffbcc8 100644
> --- a/drivers/platform/x86/panasonic-laptop.c
> +++ b/drivers/platform/x86/panasonic-laptop.c
> @@ -248,11 +248,11 @@ struct pcc_acpi {
>  	int			ac_brightness;
>  	int			dc_brightness;
>  	int			current_brightness;
> -	u32			*sinf;
>  	struct acpi_device	*device;
>  	struct input_dev	*input_dev;
>  	struct backlight_device	*backlight;
>  	struct platform_device	*platform;
> +	u32			sinf[] __counted_by(num_sifr);
>  };
>  
>  /*
> @@ -1017,21 +1017,15 @@ static int acpi_pcc_hotkey_add(struct acpi_device *device)
>  	 */
>  	num_sifr++;
>  
> -	pcc = kzalloc_obj(struct pcc_acpi);
> +	pcc = kzalloc_flex(*pcc, sinf, num_sifr);
>  	if (!pcc) {
>  		pr_err("Couldn't allocate mem for pcc");
>  		return -ENOMEM;
>  	}
>  
> -	pcc->sinf = kcalloc(num_sifr + 1, sizeof(u32), GFP_KERNEL);
> -	if (!pcc->sinf) {
> -		result = -ENOMEM;
> -		goto out_hotkey;
> -	}
> -
> +	pcc->num_sifr = num_sifr;
>  	pcc->device = device;
>  	pcc->handle = device->handle;
> -	pcc->num_sifr = num_sifr;
>  	device->driver_data = pcc;
>  	strscpy(acpi_device_name(device), ACPI_PCC_DEVICE_NAME);
>  	strscpy(acpi_device_class(device), ACPI_PCC_CLASS);
> @@ -1039,7 +1033,7 @@ static int acpi_pcc_hotkey_add(struct acpi_device *device)
>  	result = acpi_pcc_init_input(pcc);
>  	if (result) {
>  		pr_err("Error installing keyinput handler\n");
> -		goto out_sinf;
> +		goto out_hotkey;
>  	}
>  
>  	if (!acpi_pcc_retrieve_biosdata(pcc)) {
> @@ -1111,8 +1105,6 @@ static int acpi_pcc_hotkey_add(struct acpi_device *device)
>  	backlight_device_unregister(pcc->backlight);
>  out_input:
>  	input_unregister_device(pcc->input_dev);
> -out_sinf:
> -	kfree(pcc->sinf);
>  out_hotkey:
>  	kfree(pcc);
>  
> @@ -1140,7 +1132,6 @@ static void acpi_pcc_hotkey_remove(struct acpi_device *device)
>  
>  	input_unregister_device(pcc->input_dev);
>  
> -	kfree(pcc->sinf);
>  	kfree(pcc);
>  }
>  
> -- 
> 2.53.0
> 

-- 
Kees Cook

  reply	other threads:[~2026-03-20 18:32 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-20  0:49 [PATCH] platform/x86: panasonic-laptop: simplify allocation of sinf Rosen Penev
2026-03-20 18:32 ` Kees Cook [this message]
2026-03-20 23:02   ` Rosen Penev
2026-03-23  9:13     ` Ilpo Järvinen
2026-03-23 16:44       ` Rosen Penev
  -- strict thread matches above, loose matches on Subject: below --
2026-04-30 22:15 Rosen Penev
2026-05-11 17:53 ` Ilpo Järvinen

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=202603201131.55E516E@keescook \
    --to=kees@kernel.org \
    --cc=gustavoars@kernel.org \
    --cc=hansg@kernel.org \
    --cc=ilpo.jarvinen@linux.intel.com \
    --cc=kenneth.t.chan@gmail.com \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=platform-driver-x86@vger.kernel.org \
    --cc=rosenp@gmail.com \
    /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.