linux-kernel - Re: [PATCH v4] btrfs: replace deprecated 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250702182712.GA3453770@ax162>
Date: Wed, 2 Jul 2025 11:27:12 -0700
From: Nathan Chancellor <nathan@...nel.org>
To: Brahmajit Das <listout@...tout.xyz>
Cc: linux-hardening@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-btrfs@...r.kernel.org, clm@...com, josef@...icpanda.com,
	dsterba@...e.com, kees@...nel.org, ailiop@...e.com,
	mark@...mstone.com, David Sterba <dsterba@...e.cz>,
	Brahmajit Das <bdas@...e.de>
Subject: Re: [PATCH v4] btrfs: replace deprecated strcpy with strscpy

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@...e.com>
> Suggested-by: David Sterba <dsterba@...e.cz>
> Signed-off-by: Brahmajit Das <bdas@...e.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