[PATCH v4] btrfs: replace deprecated strcpy with strscpy

[PATCH v4] btrfs: replace deprecated strcpy with strscpy

[Brahmajit Das]

strcpy is deprecated due to lack of bounds checking. This patch replaces
strcpy with strscpy, the recommended alternative for null terminated
strings, to follow best practices.

There are instances where strscpy cannot be used such as where both the
source and destination are character pointers. In that instance we can
use sysfs_emit.

Link: https://github.com/KSPP/linux/issues/88
Suggested-by: Anthony Iliopoulos <ailiop@suse.com>
Suggested-by: David Sterba <dsterba@suse.cz>
Signed-off-by: Brahmajit Das <bdas@suse.de>
---

Changes in v2: using sysfs_emit instead of scnprintf.
Changes in v3: Removed string.h in xattr, since we are not using any.
fucntions from string.h and fixed length in memcpy in volumes.c
Changes in v4: As suggested by David, moving "NONE" as initial value of
buf in describe_relocation() and removed copying of "NONE" to bp in
btrfs_describe_block_groups().
---
 fs/btrfs/ioctl.c      | 2 +-
 fs/btrfs/relocation.c | 2 +-
 fs/btrfs/send.c       | 2 +-
 fs/btrfs/volumes.c    | 1 -
 fs/btrfs/xattr.c      | 3 +--
 5 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 913acef3f0a9..203f309f00b1 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -4200,7 +4200,7 @@ static int btrfs_ioctl_set_fslabel(struct file *file, void __user *arg)
 	}
 
 	spin_lock(&fs_info->super_lock);
-	strcpy(super_block->label, label);
+	strscpy(super_block->label, label);
 	spin_unlock(&fs_info->super_lock);
 	ret = btrfs_commit_transaction(trans);
 
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 02086191630d..c136552e129c 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -3880,7 +3880,7 @@ static void free_reloc_control(struct reloc_control *rc)
  */
 static void describe_relocation(struct btrfs_block_group *block_group)
 {
-	char buf[128] = {'\0'};
+	char buf[128] = "NONE";
 
 	btrfs_describe_block_groups(block_group->flags, buf, sizeof(buf));
 
diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index 2891ec4056c6..66ee9e1b1e96 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -758,7 +758,7 @@ static int send_header(struct send_ctx *sctx)
 {
 	struct btrfs_stream_header hdr;
 
-	strcpy(hdr.magic, BTRFS_SEND_STREAM_MAGIC);
+	strscpy(hdr.magic, BTRFS_SEND_STREAM_MAGIC);
 	hdr.version = cpu_to_le32(sctx->proto);
 	return write_buf(sctx->send_filp, &hdr, sizeof(hdr),
 					&sctx->send_off);
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 89835071cfea..8280474ec3d1 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -215,7 +215,6 @@ void btrfs_describe_block_groups(u64 bg_flags, char *buf, u32 size_buf)
 	u32 size_bp = size_buf;
 
 	if (!flags) {
-		strcpy(bp, "NONE");
 		return;
 	}
 
diff --git a/fs/btrfs/xattr.c b/fs/btrfs/xattr.c
index 3e0edbcf73e1..49fd8a49584a 100644
--- a/fs/btrfs/xattr.c
+++ b/fs/btrfs/xattr.c
@@ -516,8 +516,7 @@ static int btrfs_initxattrs(struct inode *inode,
 			ret = -ENOMEM;
 			break;
 		}
-		strcpy(name, XATTR_SECURITY_PREFIX);
-		strcpy(name + XATTR_SECURITY_PREFIX_LEN, xattr->name);
+		sysfs_emit(name, "%s%s", XATTR_SECURITY_PREFIX, xattr->name);
 
 		if (strcmp(name, XATTR_NAME_CAPS) == 0)
 			clear_bit(BTRFS_INODE_NO_CAP_XATTR, &BTRFS_I(inode)->runtime_flags);
-- 
2.50.0

Re: [PATCH v4] btrfs: replace deprecated strcpy with strscpy

[David Sterba]

On Fri, Jun 20, 2025 at 10:19:57PM +0530, Brahmajit Das wrote:
> strcpy is deprecated due to lack of bounds checking. This patch replaces
> strcpy with strscpy, the recommended alternative for null terminated
> strings, to follow best practices.
> 
> There are instances where strscpy cannot be used such as where both the
> source and destination are character pointers. In that instance we can
> use sysfs_emit.
> 
> Link: https://github.com/KSPP/linux/issues/88
> Suggested-by: Anthony Iliopoulos <ailiop@suse.com>
> Suggested-by: David Sterba <dsterba@suse.cz>
> Signed-off-by: Brahmajit Das <bdas@suse.de>
> ---
> 
> Changes in v2: using sysfs_emit instead of scnprintf.
> Changes in v3: Removed string.h in xattr, since we are not using any.
> fucntions from string.h and fixed length in memcpy in volumes.c
> Changes in v4: As suggested by David, moving "NONE" as initial value of
> buf in describe_relocation() and removed copying of "NONE" to bp in
> btrfs_describe_block_groups().

Sorry for the delay, added to for-next. Thanks.

Re: [PATCH v4] btrfs: replace deprecated strcpy with strscpy

[Nathan Chancellor]

Hi Brahmajit,

On Fri, Jun 20, 2025 at 10:19:57PM +0530, Brahmajit Das wrote:
> strcpy is deprecated due to lack of bounds checking. This patch replaces
> strcpy with strscpy, the recommended alternative for null terminated
> strings, to follow best practices.
> 
> There are instances where strscpy cannot be used such as where both the
> source and destination are character pointers. In that instance we can
> use sysfs_emit.
> 
> Link: https://github.com/KSPP/linux/issues/88
> Suggested-by: Anthony Iliopoulos <ailiop@suse.com>
> Suggested-by: David Sterba <dsterba@suse.cz>
> Signed-off-by: Brahmajit Das <bdas@suse.de>
...
> diff --git a/fs/btrfs/xattr.c b/fs/btrfs/xattr.c
> index 3e0edbcf73e1..49fd8a49584a 100644
> --- a/fs/btrfs/xattr.c
> +++ b/fs/btrfs/xattr.c
> @@ -516,8 +516,7 @@ static int btrfs_initxattrs(struct inode *inode,
>  			ret = -ENOMEM;
>  			break;
>  		}
> -		strcpy(name, XATTR_SECURITY_PREFIX);
> -		strcpy(name + XATTR_SECURITY_PREFIX_LEN, xattr->name);
> +		sysfs_emit(name, "%s%s", XATTR_SECURITY_PREFIX, xattr->name);
>  
>  		if (strcmp(name, XATTR_NAME_CAPS) == 0)
>  			clear_bit(BTRFS_INODE_NO_CAP_XATTR, &BTRFS_I(inode)->runtime_flags);

This change is now in -next as commit d282edfe8850 ("btrfs: replace
strcpy() with strscpy()"), where this hunk appears to causes a slew of
warnings on my arm64 systems along the lines of:

  ------------[ cut here ]------------
  invalid sysfs_emit: buf:00000000581f52ce
  WARNING: fs/sysfs/file.c:767 at sysfs_emit+0x60/0xe0, CPU#5: systemd/1
  Modules linked in:
  CPU: 5 UID: 0 PID: 1 Comm: systemd Tainted: G        W           6.16.0-rc4-next-20250702 #1 PREEMPT(voluntary)
  Tainted: [W]=WARN
  Hardware name: QEMU KVM Virtual Machine, BIOS edk2-20241117-5.fc42 11/17/2024
  pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
  pc : sysfs_emit+0x60/0xe0
  lr : sysfs_emit+0x60/0xe0
  sp : ffff80008005b840
  x29: ffff80008005b890 x28: ffff0000c0793f18 x27: ffffac7b3da61468
  x26: 0000000000400100 x25: ffffac7b3f173a88 x24: ffffac7b3f2a6480
  x23: ffff0000c0793f18 x22: ffff0000c6d4da38 x21: ffff0000c156b500
  x20: ffff0000c0e2e640 x19: ffff0000c156b500 x18: 00000000ffffffff
  x17: 65766c6f7365722d x16: 646d65747379732d x15: 0000000000000010
  x14: 0000000000000000 x13: 0000000000000008 x12: 0000000000000020
  x11: 0000000000000001 x10: 0000000000000001 x9 : ffffac7b3d2b97cc
  x8 : ffffac7b40c1aa40 x7 : ffff80008005b4a0 x6 : ffffac7b40beaa00
  x5 : ffff0003fd79c488 x4 : ffff5388bd8bc000 x3 : ffff0000c0960000
  x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff0000c0960000
  Call trace:
   sysfs_emit+0x60/0xe0 (P)
   btrfs_initxattrs+0x8c/0x148
   security_inode_init_security+0x110/0x1d8
   btrfs_xattr_security_init+0x30/0x58
   btrfs_create_new_inode+0x3cc/0xc60
   btrfs_create_common+0xdc/0x148
   btrfs_mkdir+0x7c/0xc0
   vfs_mkdir+0x1a0/0x290
   do_mkdirat+0x150/0x190
   __arm64_sys_mkdirat+0x54/0xb0
   invoke_syscall.constprop.0+0x64/0xe8
   el0_svc_common.constprop.0+0x40/0xe8
   do_el0_svc+0x24/0x38
   el0_svc+0x3c/0x170
   el0t_64_sync_handler+0x10c/0x138
   el0t_64_sync+0x1b0/0x1b8
  ---[ end trace 0000000000000000 ]---

It looks like the offset_in_page(buf) part of the WARN() in
sysfs_emit() gets triggered with this, presumably because kmalloc()
returns something that is not page aligned like sysfs_emit() requires?

Cheers,
Nathan

Re: [PATCH v4] btrfs: replace deprecated strcpy with strscpy

[Brahmajit Das]

On 02.07.2025 11:27, Nathan Chancellor wrote:
> Hi Brahmajit,
> 
> On Fri, Jun 20, 2025 at 10:19:57PM +0530, Brahmajit Das wrote:
... snip ...
> 
> It looks like the offset_in_page(buf) part of the WARN() in
> sysfs_emit() gets triggered with this, presumably because kmalloc()
> returns something that is not page aligned like sysfs_emit() requires?
> 
> Cheers,
> Nathan

Hey Nathan, thanks for reporting this. From the QEMU logs this looks
like on ARM64. Unfortunately I didn't boot test on arm due to not having
the hardware. I'll setup a qemu test env. for ARM and get back.

Sorry I'm new to kernel development.
-- 
Regards,
listout

Re: [PATCH v4] btrfs: replace deprecated strcpy with strscpy

[Brahmajit Das]

On 02.07.2025 11:27, Nathan Chancellor wrote:
> Hi Brahmajit,
> 
> On Fri, Jun 20, 2025 at 10:19:57PM +0530, Brahmajit Das wrote:
...
> 
> This change is now in -next as commit d282edfe8850 ("btrfs: replace
> strcpy() with strscpy()"), where this hunk appears to causes a slew of
> warnings on my arm64 systems along the lines of:
> 
...
> 
> It looks like the offset_in_page(buf) part of the WARN() in
> sysfs_emit() gets triggered with this, presumably because kmalloc()
> returns something that is not page aligned like sysfs_emit() requires?
> 
> Cheers,
> Nathan

Nathan, can you help me with providing a bit more info to debug this. I
set up qemu aarch64 env with btrfs but couldn't reproduce this issue by
boot test. Basically trying to understand what workflow triggered this.

You can find my kernel config, dmesg log and boot logs here:
https://gist.github.com/listout/de8b6efa6ddb02805b5886f35c3f73d4

Not to mention I'm very much open to suggestion from other btrfs
developers as well.
-- 
Regards,
listout

Re: [PATCH v4] btrfs: replace deprecated strcpy with strscpy

[David Sterba]

On Thu, Jul 03, 2025 at 06:08:24AM +0530, Brahmajit Das wrote:
> On 02.07.2025 11:27, Nathan Chancellor wrote:
> > Hi Brahmajit,
> > 
> > On Fri, Jun 20, 2025 at 10:19:57PM +0530, Brahmajit Das wrote:
> ...
> > 
> > This change is now in -next as commit d282edfe8850 ("btrfs: replace
> > strcpy() with strscpy()"), where this hunk appears to causes a slew of
> > warnings on my arm64 systems along the lines of:
> > 
> ...
> > 
> > It looks like the offset_in_page(buf) part of the WARN() in
> > sysfs_emit() gets triggered with this, presumably because kmalloc()
> > returns something that is not page aligned like sysfs_emit() requires?
>
> Nathan, can you help me with providing a bit more info to debug this. I
> set up qemu aarch64 env with btrfs but couldn't reproduce this issue by
> boot test. Basically trying to understand what workflow triggered this.
> 
> You can find my kernel config, dmesg log and boot logs here:
> https://gist.github.com/listout/de8b6efa6ddb02805b5886f35c3f73d4

sysfs_emit() is wrapper for scnprintf with implicit buffer length, so
we can use that instead. I'll fix it in for-next.