linux-kernel - Re: [PATCH] driver core: Fix size calculation of symlink name for devlink_(add|remove)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAGETcx8YwD-cWYFJ72rfSfmrnoY=rv9oc_2KCK9_AF34Evw7wg@mail.gmail.com>
Date: Mon, 8 Jul 2024 15:43:26 -0700
From: Saravana Kannan <saravanak@...gle.com>
To: Zijun Hu <zijun_hu@...oud.com>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>, "Rafael J. Wysocki" <rafael@...nel.org>, 
	linux-kernel@...r.kernel.org, Zijun Hu <quic_zijuhu@...cinc.com>
Subject: Re: [PATCH] driver core: Fix size calculation of symlink name for devlink_(add|remove)_symlinks()

On Sun, Jul 7, 2024 at 6:24 AM Zijun Hu <zijun_hu@...oud.com> wrote:
>
> From: Zijun Hu <quic_zijuhu@...cinc.com>
>
> devlink_(add|remove)_symlinks() wants to kzalloc() memory to save symlink
> name for either supplier or consumer, but forget to consider consumer
> prefix when calclulate memory size, fixed by considering prefix for both
> supplier and consumer.

No, I didn't forget to take "consumer" into account :) Both supplier
and consumer are the same length. So I didn't bother doing both. I
don't see a point behind this patch.

>
> Fixes: 287905e68dd2 ("driver core: Expose device link details in sysfs")

It's definitely not "Fixing" anything because nothing is broken.

Nack.

If you really want this in, remove this tag and send it again. I won't
ack or review it though as I don't think it adds much value. Greg can
take it if he thinks he likes it.

-Saravana

> Signed-off-by: Zijun Hu <quic_zijuhu@...cinc.com>
> ---
>  drivers/base/core.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 2b4c0624b704..f14cfe5c97b7 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -572,7 +572,7 @@ static int devlink_add_symlinks(struct device *dev)
>         len = max(strlen(dev_bus_name(sup)) + strlen(dev_name(sup)),
>                   strlen(dev_bus_name(con)) + strlen(dev_name(con)));
>         len += strlen(":");
> -       len += strlen("supplier:") + 1;
> +       len += max(strlen("supplier:"), strlen("consumer:")) + 1;
>         buf = kzalloc(len, GFP_KERNEL);
>         if (!buf)
>                 return -ENOMEM;
> @@ -623,7 +623,7 @@ static void devlink_remove_symlinks(struct device *dev)
>         len = max(strlen(dev_bus_name(sup)) + strlen(dev_name(sup)),
>                   strlen(dev_bus_name(con)) + strlen(dev_name(con)));
>         len += strlen(":");
> -       len += strlen("supplier:") + 1;
> +       len += max(strlen("supplier:"), strlen("consumer:")) + 1;
>         buf = kzalloc(len, GFP_KERNEL);
>         if (!buf) {
>                 WARN(1, "Unable to properly free device link symlinks!\n");
>
> ---
> base-commit: c6653f49e4fd3b0d52c12a1fc814d6c5b234ea15
> change-id: 20240707-devlink_fix-0fa46dedfe95
>
> Best regards,
> --
> Zijun Hu <quic_zijuhu@...cinc.com>
>