mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/3] ksmbd: fix three named stream bugs
@ 2026-10-08 15:25 DaeMyung Kang
  2026-10-08 15:25 ` [PATCH 1/3] ksmbd: return end of file for reads past a named stream's data DaeMyung Kang
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: DaeMyung Kang @ 2026-10-08 15:25 UTC (permalink / raw)
  To: Namjae Jeon
  Cc: Sergey Senozhatsky, Tom Talpey, ChenXiaoSong, linux-cifs,
	linux-kernel, DaeMyung Kang

Hi Namjae,

This series fixes three independent named stream bugs:

  1. A read at or beyond the end of a non-empty stream returns
     STATUS_INVALID_PARAMETER instead of STATUS_END_OF_FILE.
  2. Opening an existing stream with different letter case keeps the
     client's spelling in the handle. Later writes and removal can
     target a different xattr from the one that was opened.
  3. A write-only FILE_OPEN_IF on an existing stream can replace its
     value with an empty xattr when the value-length lookup fails with
     -EACCES.

The series applies to ksmbd-for-next at 5d2b1ab54e9a ("ksmbd: fix
named stream write and EOF handling"). Each fix is in a separate
patch, with its own Fixes tag and stable Cc.

I built the resulting kernel with CONFIG_SMB_SERVER=y and tested it
through SMB2.1 in a QEMU guest. Reads at and past stream EOF returned
STATUS_END_OF_FILE. A stream created as Foo was read, modified and
removed through foo, with one named stream in the listing. On a 0222
file, a write-only user opening an existing stream with FILE_OPEN_IF
received STATUS_ACCESS_DENIED and the original value remained intact;
without patch 3, the same sequence emptied the stream. The same user
could still create a new stream.

Patch 2 has a minor textual conflict in linux-next with the VFS tree's
const mnt_idmap conversion. The resolution keeps both the const
qualifier and the new actual_name argument.

Concurrent creation of case-variant stream names remains a separate
issue.

DaeMyung Kang (3):
  ksmbd: return end of file for reads past a named stream's data
  ksmbd: use the existing xattr name for a named stream
  ksmbd: preserve unreadable named streams on open

 fs/smb/server/smb2pdu.c | 10 ++++++----
 fs/smb/server/vfs.c     | 17 +++++++++++++----
 fs/smb/server/vfs.h     |  2 +-
 3 files changed, 20 insertions(+), 9 deletions(-)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/3] ksmbd: return end of file for reads past a named stream's data
  2026-10-08 15:25 [PATCH 0/3] ksmbd: fix three named stream bugs DaeMyung Kang
@ 2026-10-08 15:25 ` DaeMyung Kang
  2026-10-08 15:25 ` [PATCH 2/3] ksmbd: use the existing xattr name for a named stream DaeMyung Kang
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: DaeMyung Kang @ 2026-10-08 15:25 UTC (permalink / raw)
  To: Namjae Jeon
  Cc: Sergey Senozhatsky, Tom Talpey, ChenXiaoSong, linux-cifs,
	linux-kernel, DaeMyung Kang, stable

A READ on a non-empty named stream at or beyond its end fails with
STATUS_INVALID_PARAMETER. ksmbd_vfs_stream_read() returns -EINVAL when
the offset is not below the stream length, and smb2_read() maps -EINVAL
to that status.

smb2_read() already turns a zero-byte read into STATUS_END_OF_FILE, the
status a regular file returns at its end, and an empty stream already
takes that path. Return 0 instead of -EINVAL so that non-empty streams
do the same. Clients that read a stream until end of file otherwise get
an error once they have read all of its data.

The buffered copychunk path converts a zero-byte source read to -EIO,
as it does for a regular file that shrinks during copying.

Fixes: 2ae1a6cc4302 ("cifsd: fix potential read overflow in ksmbd_vfs_stream_read()")
Cc: stable@vger.kernel.org
Signed-off-by: DaeMyung Kang <charsyam@gmail.com>
---
 fs/smb/server/vfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/smb/server/vfs.c b/fs/smb/server/vfs.c
index 74cd8007a..ae9f9a693 100644
--- a/fs/smb/server/vfs.c
+++ b/fs/smb/server/vfs.c
@@ -269,7 +269,7 @@ static int ksmbd_vfs_stream_read(struct ksmbd_file *fp, char *buf, loff_t *pos,
 		return (int)v_len;
 
 	if (v_len <= *pos) {
-		count = -EINVAL;
+		count = 0;
 		goto free_buf;
 	}
 
-- 
2.43.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 2/3] ksmbd: use the existing xattr name for a named stream
  2026-10-08 15:25 [PATCH 0/3] ksmbd: fix three named stream bugs DaeMyung Kang
  2026-10-08 15:25 ` [PATCH 1/3] ksmbd: return end of file for reads past a named stream's data DaeMyung Kang
@ 2026-10-08 15:25 ` DaeMyung Kang
  2026-10-08 15:25 ` [PATCH 3/3] ksmbd: preserve unreadable named streams on open DaeMyung Kang
  2026-10-09  2:24 ` [PATCH 0/3] ksmbd: fix three named stream bugs Namjae Jeon
  3 siblings, 0 replies; 5+ messages in thread
From: DaeMyung Kang @ 2026-10-08 15:25 UTC (permalink / raw)
  To: Namjae Jeon
  Cc: Sergey Senozhatsky, Tom Talpey, ChenXiaoSong, linux-cifs,
	linux-kernel, DaeMyung Kang, stable

SMB stream names are case-insensitive, but ksmbd stores each named
stream in an xattr whose name is case-sensitive. Opening an existing
stream finds its xattr with a case-insensitive match, yet the handle
keeps the name as the client spelled it, and every later operation that
needs an exact name uses that spelling:

 - a WRITE or an end-of-file change creates a second xattr, so a stream
   created as "Foo" and written through "foo" ends up stored twice, and
   which value a later lookup returns depends on xattr list order;
 - delete-on-close and delete-pending removal can fail with -ENODATA,
   or remove only a case-variant copy and leave the original;
 - the share mode check compares stream names with strcmp(), so two
   opens of the same stream that differ only in case do not conflict.

Have ksmbd_vfs_casexattr_len() copy the matching xattr's name into the
handle when the stream is opened, so later operations use the name
already on disk. The match must have the same length as the name it
replaces; the stream lookup passes a length that includes the
terminating NUL, which already guarantees that, and the check makes the
copy safe for any caller. A stream that does not exist yet keeps the
client's spelling, as before.

smb2_rename() compares and looks up fp->stream.name case-insensitively,
so it behaves the same with either spelling.

Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")
Cc: stable@vger.kernel.org
Signed-off-by: DaeMyung Kang <charsyam@gmail.com>
---
 fs/smb/server/smb2pdu.c | 8 ++++----
 fs/smb/server/vfs.c     | 9 +++++++--
 fs/smb/server/vfs.h     | 2 +-
 3 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c
index 12a43efdb..4d43de74f 100644
--- a/fs/smb/server/smb2pdu.c
+++ b/fs/smb/server/smb2pdu.c
@@ -3341,7 +3341,7 @@ static int smb2_set_ea(struct smb2_ea_info *eabuf, unsigned int buf_len,
 						     path->dentry,
 						     attr_name,
 						     XATTR_USER_PREFIX_LEN +
-						     eabuf->EaNameLength);
+						     eabuf->EaNameLength, NULL);
 
 			/* delete the EA only when it exits */
 			if (rc > 0) {
@@ -3413,11 +3413,11 @@ static noinline int smb2_set_stream_name_xattr(const struct path *path,
 	fp->stream.name = xattr_stream_name;
 	fp->stream.size = xattr_stream_size;
 
-	/* Check if there is stream prefix in xattr space */
+	/* Keep the existing xattr's case for subsequent writes and removal. */
 	rc = ksmbd_vfs_casexattr_len(idmap,
 				     path->dentry,
 				     xattr_stream_name,
-				     xattr_stream_size);
+				     xattr_stream_size, xattr_stream_name);
 	if (rc >= 0)
 		return 0;
 
@@ -3469,7 +3469,7 @@ static loff_t ksmbd_stream_eof(struct ksmbd_file *fp)
 	ssize_t slen = ksmbd_vfs_casexattr_len(file_mnt_idmap(fp->filp),
 					       fp->filp->f_path.dentry,
 					       fp->stream.name,
-					       fp->stream.size);
+					       fp->stream.size, NULL);
 	return slen < 0 ? 0 : (loff_t)slen;
 }
 
diff --git a/fs/smb/server/vfs.c b/fs/smb/server/vfs.c
index ae9f9a693..7020b7de3 100644
--- a/fs/smb/server/vfs.c
+++ b/fs/smb/server/vfs.c
@@ -1946,7 +1946,7 @@ int ksmbd_vfs_fill_dentry_attrs(struct ksmbd_work *work,
 
 ssize_t ksmbd_vfs_casexattr_len(struct mnt_idmap *idmap,
 				struct dentry *dentry, char *attr_name,
-				int attr_name_len)
+				int attr_name_len, char *actual_name)
 {
 	char *name, *xattr_list = NULL;
 	ssize_t value_len = -ENOENT, xattr_list_len;
@@ -1960,8 +1960,13 @@ ssize_t ksmbd_vfs_casexattr_len(struct mnt_idmap *idmap,
 		ksmbd_debug(VFS, "%s, len %zd\n", name, strlen(name));
 		if (strncasecmp(attr_name, name, attr_name_len))
 			continue;
+		if (actual_name && strlen(name) + 1 != attr_name_len)
+			continue;
 
 		value_len = ksmbd_vfs_xattr_len(idmap, dentry, name);
+		/* The caller provides attr_name_len bytes for the actual name. */
+		if (value_len >= 0 && actual_name)
+			memcpy(actual_name, name, attr_name_len);
 		break;
 	}
 
@@ -2116,7 +2121,7 @@ int ksmbd_vfs_copy_file_ranges(struct ksmbd_work *work,
 		src_file_size = ksmbd_vfs_casexattr_len(
 				file_mnt_idmap(src_fp->filp),
 				src_fp->filp->f_path.dentry,
-				src_fp->stream.name, src_fp->stream.size);
+				src_fp->stream.name, src_fp->stream.size, NULL);
 		revert_creds(saved_cred);
 		if (src_file_size < 0)
 			return src_file_size;
diff --git a/fs/smb/server/vfs.h b/fs/smb/server/vfs.h
index ef3ab3f18..dedb10331 100644
--- a/fs/smb/server/vfs.h
+++ b/fs/smb/server/vfs.h
@@ -116,7 +116,7 @@ ssize_t ksmbd_vfs_getcasexattr(struct mnt_idmap *idmap,
 			       int attr_name_len, char **attr_value);
 ssize_t ksmbd_vfs_casexattr_len(struct mnt_idmap *idmap,
 				struct dentry *dentry, char *attr_name,
-				int attr_name_len);
+				int attr_name_len, char *actual_name);
 int ksmbd_vfs_setxattr(struct mnt_idmap *idmap,
 		       const struct path *path, const char *attr_name,
 		       void *attr_value, size_t attr_size, int flags,
-- 
2.43.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 3/3] ksmbd: preserve unreadable named streams on open
  2026-10-08 15:25 [PATCH 0/3] ksmbd: fix three named stream bugs DaeMyung Kang
  2026-10-08 15:25 ` [PATCH 1/3] ksmbd: return end of file for reads past a named stream's data DaeMyung Kang
  2026-10-08 15:25 ` [PATCH 2/3] ksmbd: use the existing xattr name for a named stream DaeMyung Kang
@ 2026-10-08 15:25 ` DaeMyung Kang
  2026-10-09  2:24 ` [PATCH 0/3] ksmbd: fix three named stream bugs Namjae Jeon
  3 siblings, 0 replies; 5+ messages in thread
From: DaeMyung Kang @ 2026-10-08 15:25 UTC (permalink / raw)
  To: Namjae Jeon
  Cc: Sergey Senozhatsky, Tom Talpey, ChenXiaoSong, linux-cifs,
	linux-kernel, DaeMyung Kang, stable

A write-only client can open an existing named stream with FILE_OPEN_IF
without permission to read the underlying file. The case-insensitive
lookup finds the xattr name, but querying its value length then fails
with -EACCES. smb2_set_stream_name_xattr() treats every negative result
as a missing stream and replaces the existing xattr with an empty value.

A failed xattr list is not proof that the stream is absent either:
for example, -ENOMEM can lead to the same empty replacement. Preserve
listing errors and create a stream only for -ENOENT or -ENODATA. The
latter can occur if the xattr is removed between listing its name and
querying its value. Return other lookup errors without changing the
stream.

A write-only FILE_OPEN now reports access denied instead of name not
found. FILE_OVERWRITE_IF and FILE_SUPERSEDE on an unreadable stream
also fail instead of appearing to replace it. Stream writes and EOF
changes already need to read the old value, so such handles cannot
update the stream after opening either.

Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")
Cc: stable@vger.kernel.org
Signed-off-by: DaeMyung Kang <charsyam@gmail.com>
---
 fs/smb/server/smb2pdu.c | 2 ++
 fs/smb/server/vfs.c     | 6 +++++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c
index 4d43de74f..52a19871d 100644
--- a/fs/smb/server/smb2pdu.c
+++ b/fs/smb/server/smb2pdu.c
@@ -3420,6 +3420,8 @@ static noinline int smb2_set_stream_name_xattr(const struct path *path,
 				     xattr_stream_size, xattr_stream_name);
 	if (rc >= 0)
 		return 0;
+	if (rc != -ENOENT && rc != -ENODATA)
+		return rc;
 
 	if (fp->cdoption == FILE_OPEN_LE) {
 		if (!strcmp(stream_name, "AFP_AfpInfo") &&
diff --git a/fs/smb/server/vfs.c b/fs/smb/server/vfs.c
index 7020b7de3..3fa0e26e5 100644
--- a/fs/smb/server/vfs.c
+++ b/fs/smb/server/vfs.c
@@ -1952,7 +1952,11 @@ ssize_t ksmbd_vfs_casexattr_len(struct mnt_idmap *idmap,
 	ssize_t value_len = -ENOENT, xattr_list_len;
 
 	xattr_list_len = ksmbd_vfs_listxattr(dentry, &xattr_list);
-	if (xattr_list_len <= 0)
+	if (xattr_list_len < 0) {
+		value_len = xattr_list_len;
+		goto out;
+	}
+	if (!xattr_list_len)
 		goto out;
 
 	for (name = xattr_list; name - xattr_list < xattr_list_len;
-- 
2.43.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 0/3] ksmbd: fix three named stream bugs
  2026-10-08 15:25 [PATCH 0/3] ksmbd: fix three named stream bugs DaeMyung Kang
                   ` (2 preceding siblings ...)
  2026-10-08 15:25 ` [PATCH 3/3] ksmbd: preserve unreadable named streams on open DaeMyung Kang
@ 2026-10-09  2:24 ` Namjae Jeon
  3 siblings, 0 replies; 5+ messages in thread
From: Namjae Jeon @ 2026-10-09  2:24 UTC (permalink / raw)
  To: DaeMyung Kang
  Cc: Sergey Senozhatsky, Tom Talpey, ChenXiaoSong, linux-cifs, linux-kernel

On Fri, Oct 9, 2026 at 12:25 AM DaeMyung Kang <charsyam@gmail.com> wrote:
>
> Hi Namjae,
>
> This series fixes three independent named stream bugs:
>
>   1. A read at or beyond the end of a non-empty stream returns
>      STATUS_INVALID_PARAMETER instead of STATUS_END_OF_FILE.
>   2. Opening an existing stream with different letter case keeps the
>      client's spelling in the handle. Later writes and removal can
>      target a different xattr from the one that was opened.
>   3. A write-only FILE_OPEN_IF on an existing stream can replace its
>      value with an empty xattr when the value-length lookup fails with
>      -EACCES.
>
> The series applies to ksmbd-for-next at 5d2b1ab54e9a ("ksmbd: fix
> named stream write and EOF handling"). Each fix is in a separate
> patch, with its own Fixes tag and stable Cc.
>
> I built the resulting kernel with CONFIG_SMB_SERVER=y and tested it
> through SMB2.1 in a QEMU guest. Reads at and past stream EOF returned
> STATUS_END_OF_FILE. A stream created as Foo was read, modified and
> removed through foo, with one named stream in the listing. On a 0222
> file, a write-only user opening an existing stream with FILE_OPEN_IF
> received STATUS_ACCESS_DENIED and the original value remained intact;
> without patch 3, the same sequence emptied the stream. The same user
> could still create a new stream.
>
> Patch 2 has a minor textual conflict in linux-next with the VFS tree's
> const mnt_idmap conversion. The resolution keeps both the const
> qualifier and the new actual_name argument.
>
> Concurrent creation of case-variant stream names remains a separate
> issue.
>
> DaeMyung Kang (3):
>   ksmbd: return end of file for reads past a named stream's data
>   ksmbd: use the existing xattr name for a named stream
>   ksmbd: preserve unreadable named streams on open
Applied them to #ksmbd-for-next.
Thanks!

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-10-09  2:24 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-08 15:25 [PATCH 0/3] ksmbd: fix three named stream bugs DaeMyung Kang
2026-10-08 15:25 ` [PATCH 1/3] ksmbd: return end of file for reads past a named stream's data DaeMyung Kang
2026-10-08 15:25 ` [PATCH 2/3] ksmbd: use the existing xattr name for a named stream DaeMyung Kang
2026-10-08 15:25 ` [PATCH 3/3] ksmbd: preserve unreadable named streams on open DaeMyung Kang
2026-10-09  2:24 ` [PATCH 0/3] ksmbd: fix three named stream bugs Namjae Jeon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®