linux-kernel - Re: [PATCH] most: replace strcpy() with strscpy()

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] [day] [month] [year] [list]

Message-ID: <77d800c3-d578-44cc-b17c-fffc7cd05736@microchip.com>
Date: Mon, 6 Jan 2025 05:22:16 +0000
From: <Parthiban.Veerasooran@...rochip.com>
To: <abdul.rahim@...ahoo.com>
CC: <linux-kernel@...r.kernel.org>, <Christian.Gromm@...rochip.com>
Subject: Re: [PATCH] most: replace strcpy() with strscpy()

Hi,

On 05/01/25 11:42 pm, Abdul Rahim wrote:
> [You don't often get email from abdul.rahim@...ahoo.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> strcpy() performs no bounds checking on the destination buffer. This
> could result in linear overflows beyond the end of the buffer, leading
> to all kinds of misbehaviors.[1] The return value is not used here so
> direct replacement is safe.
> 
> [1]: https://docs.kernel.org/process/deprecated.html#strcpy
Both destination and source buffers of all the below strcpy() are 
created and handled within the driver. Here the boundary of destination 
and source buffers are defined and handled properly. Destination buffer 
size is 80 and the source strings are fixed within that length. Moreover 
those source strings are validated/checked before doing strcpy(). So I 
don't see any valid reason to replace strcpy() with strscpy(). If those 
buffers are from an user and if the driver doesn't take care of the 
boundary or validity of those buffers then there might be a possibility 
of boundary violation. But that is not the case here.

Best regards,
Parthiban V
> 
> Signed-off-by: Abdul Rahim <abdul.rahim@...ahoo.com>
> ---
>   drivers/most/configfs.c | 16 ++++++++--------
>   1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/most/configfs.c b/drivers/most/configfs.c
> index 36d8c917f65f..687e63669ccd 100644
> --- a/drivers/most/configfs.c
> +++ b/drivers/most/configfs.c
> @@ -170,7 +170,7 @@ static ssize_t mdev_link_direction_store(struct config_item *item,
>          if (!sysfs_streq(page, "dir_rx") && !sysfs_streq(page, "rx") &&
>              !sysfs_streq(page, "dir_tx") && !sysfs_streq(page, "tx"))
>                  return -EINVAL;
> -       strcpy(mdev_link->direction, page);
> +       strscpy(mdev_link->direction, page);
>          strim(mdev_link->direction);
>          return count;
>   }
> @@ -189,7 +189,7 @@ static ssize_t mdev_link_datatype_store(struct config_item *item,
>              !sysfs_streq(page, "sync") && !sysfs_streq(page, "isoc") &&
>              !sysfs_streq(page, "isoc_avp"))
>                  return -EINVAL;
> -       strcpy(mdev_link->datatype, page);
> +       strscpy(mdev_link->datatype, page);
>          strim(mdev_link->datatype);
>          return count;
>   }
> @@ -438,12 +438,12 @@ static struct config_item *most_common_make_item(struct config_group *group,
>                                     &mdev_link_type);
> 
>          if (!strcmp(group->cg_item.ci_namebuf, "most_cdev"))
> -               strcpy(mdev_link->comp, "cdev");
> +               strscpy(mdev_link->comp, "cdev");
>          else if (!strcmp(group->cg_item.ci_namebuf, "most_net"))
> -               strcpy(mdev_link->comp, "net");
> +               strscpy(mdev_link->comp, "net");
>          else if (!strcmp(group->cg_item.ci_namebuf, "most_video"))
> -               strcpy(mdev_link->comp, "video");
> -       strcpy(mdev_link->name, name);
> +               strscpy(mdev_link->comp, "video");
> +       strscpy(mdev_link->name, name);
>          return &mdev_link->item;
>   }
> 
> @@ -532,8 +532,8 @@ static struct config_item *most_snd_grp_make_item(struct config_group *group,
> 
>          config_item_init_type_name(&mdev_link->item, name, &mdev_link_type);
>          mdev_link->create_link = false;
> -       strcpy(mdev_link->name, name);
> -       strcpy(mdev_link->comp, "sound");
> +       strscpy(mdev_link->name, name);
> +       strscpy(mdev_link->comp, "sound");
>          return &mdev_link->item;
>   }
> 
> --
> 2.43.0
>