Re: [PATCH v3 4/4] mm: huge_memory: Use strscpy() instead of strcpy()

[Maíra Canal]

Hi Barry,

On 30/10/24 20:07, Barry Song wrote:
> On Thu, Oct 31, 2024 at 2:03 AM Maíra Canal <mcanal@xxxxxxxxxx> wrote:
> > Replace strcpy() with strscpy() in mm/huge_memory.c
> >
> > strcpy() has been deprecated because it is generally unsafe, so help to
> > eliminate it from the kernel source.
> >
> > Link: https://github.com/KSPP/linux/issues/88
> > Signed-off-by: Maíra Canal <mcanal@xxxxxxxxxx>
> > ---
> >   mm/huge_memory.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> > index f92068864469..8f41a694433c 100644
> > --- a/mm/huge_memory.c
> > +++ b/mm/huge_memory.c
> > @@ -989,7 +989,7 @@ static int __init setup_thp_anon(char *str)
> >
> >          if (!str || strlen(str) + 1 > PAGE_SIZE)
> >                  goto err;
> > -       strcpy(str_dup, str);
> > +       strscpy(str_dup, str);
>
> What is the difference between strcpy and strscpy without a size parameter?
>
> we have already a check and goto err. strcpy() is entirely safe.
>           if (!str || strlen(str) + 1 > PAGE_SIZE)
>                   goto err;
>
> My understanding is that we don't need this patch.

strcpy() is a deprecated interface [1]. From the GitHub issue I linked
in the commit description, Kees states: "A lot of kernel code is still
using strcpy(). While the CONFIG_FORTIFY_SOURCE wrapper macros tend to
make its use mostly safe, it would be nice to eliminate the function
from the kernel entirely."

[1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strcpy

Best Regards,
- Maíra

> >          always = huge_anon_orders_always;
> >          madvise = huge_anon_orders_madvise;
> > @@ -4175,7 +4175,7 @@ static ssize_t split_huge_pages_write(struct file *file, const char __user *buf,
> >
> >                  tok = strsep(&buf, ",");
> >                  if (tok) {
> > -                       strcpy(file_path, tok);
> > +                       strscpy(file_path, tok);
> >                  } else {
> >                          ret = -EINVAL;
> >                          goto out;
> > --
> > 2.46.2
>
> Thanks
> barry