linux-kernel - [PATCH v2] ceph: Use strscpy() instead of strcpy()

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

Message-ID: <20241115112419.11137-1-abdul.rahim@myyahoo.com>
Date: Fri, 15 Nov 2024 16:54:19 +0530
From: Abdul Rahim <abdul.rahim@...ahoo.com>
To: xiubli@...hat.com,
	idryomov@...il.com
Cc: ceph-devel@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Abdul Rahim <abdul.rahim@...ahoo.com>
Subject: [PATCH v2] ceph: Use strscpy() instead of strcpy()

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]

this fixes checkpatch warning:
    WARNING: Prefer strscpy over strcpy

[1] : https://www.kernel.org/doc/html/latest/process/deprecated.html#strcpy
Signed-off-by: Abdul Rahim <abdul.rahim@...ahoo.com>
---
Changes since v1:
- Added third parameter in strscpy()
- Added comment to explain where the limit `NAME_MAX+1` is comming from
  as suggested by Christophe JAILLET <christophe.jaillet@...adoo.fr>

Link to v1: https://lore.kernel.org/lkml/20241111221037.92853-1-abdul.rahim@myyahoo.com/

The function __get_snap_name() is assigned to .get_name() from 
struct export_operations, when `ceph_snap(inode) != CEPH_NOSNAP`.
`struct export_operations` is comming from `include/linux/exportfs.h`,
and according to [1], the operation get_name assumes that the variable
`name` is pointing to a buffer of size NAME_MAX+1

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/exportfs.h?h=v6.12-rc7#n203

 fs/ceph/export.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/fs/ceph/export.c b/fs/ceph/export.c
index 44451749c544..96421f2b6cec 100644
--- a/fs/ceph/export.c
+++ b/fs/ceph/export.c
@@ -452,7 +452,11 @@ static int __get_snap_name(struct dentry *parent, char *name,
 		goto out;
 	if (ceph_snap(inode) == CEPH_SNAPDIR) {
 		if (ceph_snap(dir) == CEPH_NOSNAP) {
-			strcpy(name, fsc->mount_options->snapdir_name);
+			/* .get_name() from struct export_operations assumes
+			 * that its 'name' parameter is pointing to a 
+			 * NAME_MAX+1 sized buffer */
+			strscpy(name, fsc->mount_options->snapdir_name,
+					NAME_MAX+1);
 			err = 0;
 		}
 		goto out;
-- 
2.43.0