linux-kernel - Re: [PATCH v2] xfs: Replace strncpy with memcpy

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 for Android: free password hash cracker in your pocket

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

Message-ID: <hi2bosmnbbqbsrxydjqh4w7ovzggfdvpafubqbzdovuwzwqlfh@z2wbwjaqizzk>
Date: Thu, 21 Aug 2025 11:39:17 +0200
From: Carlos Maiolino <cem@...nel.org>
To: Marcelo Moreira <marcelomoreira1905@...il.com>
Cc: linux-xfs@...r.kernel.org, linux-kernel@...r.kernel.org, 
	skhan@...uxfoundation.org, linux-kernel-mentees@...ts.linuxfoundation.org
Subject: Re: [PATCH v2] xfs: Replace strncpy with memcpy

Hi.

On Sun, Aug 17, 2025 at 12:50:41PM -0300, Marcelo Moreira wrote:
> Following a suggestion from Dave and everyone who contributed to v1, this
> changes modernizes the code by aligning it with current kernel best practices.
> It improves code clarity and consistency, as strncpy is deprecated as explained
> in Documentation/process/deprecated.rst. Furthermore, this change was tested
> by xfstests

> and as it was not an easy task I decided to document on my blog
> the step by step of how I did it https://meritissimo1.com/blog/2-xfs-tests :).

The above line does not belong to the commit description. I'm glad
you've tested everything as we suggested and got to the point to run
xfstests which indeed is not a single-click button. But the patch
description is not a place to document it.

> 
> This change does not alter the functionality or introduce any behavioral
> changes.

^ This should be in the description...

> 
> Changes include:
>  - Replace strncpy with memcpy.

^ This is unnecessary. It's a plain copy/paste from the subject, no need
to write yet again.

> 
> ---

^ Keep a single --- in the patch... This is used as metadata, everything
  below the first --- is ignored by git.

> Changelog:
> 
> Changes since v1:
> - Replace strncpy with memcpy instead of strscpy.
> - The change was tested using xfstests.
> 
> Link to v1: https://lore.kernel.org/linux-kernel-mentees/CAPZ3m_jXwp1FfsvtR2s3nwATT3fER=Mc6qj+GzKuUhY5tjQFNQ@mail.gmail.com/T/#t
> 

^ All those Changelog metadata should be below the ---, so they don't
get into the commit message, but...

> Suggested-by: Dave Chinner <david@...morbit.com>
> Signed-off-by: Marcelo Moreira <marcelomoreira1905@...il.com>

^ Those should be before the '---' otherwise, as I mentioned, git will
ignore those.

> ---
>  fs/xfs/scrub/symlink_repair.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/scrub/symlink_repair.c b/fs/xfs/scrub/symlink_repair.c
> index 953ce7be78dc..5902398185a8 100644
> --- a/fs/xfs/scrub/symlink_repair.c
> +++ b/fs/xfs/scrub/symlink_repair.c
> @@ -185,7 +185,7 @@ xrep_symlink_salvage_inline(
>  		return 0;
> 
>  	nr = min(XFS_SYMLINK_MAXLEN, xfs_inode_data_fork_size(ip));
> -	strncpy(target_buf, ifp->if_data, nr);
> +	memcpy(target_buf, ifp->if_data, nr);
>  	return nr;

The change looks fine. Once you fix the above points:

Reviewed-by: Carlos Maiolino <cmaiolino@...hat.com>

Cheers,
Carlos

>  }
> 
> --
> 2.50.1
> 
>