* [RFC PATCH 1/6] ksmbd: don't ignore the owner and group of a descriptor without a DACL
@ 2026-11-10 9:02 Jan Koester
0 siblings, 0 replies; 2+ messages in thread
From: Jan Koester @ 2026-11-10 9:02 UTC (permalink / raw)
To: linkinjeon, smfrench
Cc: senozhatsky, tom, chenxiaosong, brauner, linux-cifs,
linux-fsdevel, samba-technical, linux-kernel
parse_sec_desc() returned before looking at the owner and group SIDs
when the descriptor had no DACL_PRESENT, so an SMB2 SET_INFO with only
OWNER_SECURITY_INFORMATION or GROUP_SECURITY_INFORMATION -- what Windows
sends to take ownership, and smbcacls -C/-G -- did nothing and still
reported success.
Map the owner and group in any case and parse the DACL only if there is
one. set_info_sec() then changes the mode and the POSIX ACLs only for a
descriptor with a DACL, and leaves the stored descriptor alone without
one: replacing it would drop its DACL, and the owner shown comes from
the inode anyway. This also covers the SD buffer create context, so its
type_check argument goes away.
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Jan Koester <jan.koester@tuxist.de>
---
fs/smb/server/smb2pdu.c | 4 ++--
fs/smb/server/smbacl.c | 39 ++++++++++++++++++++++++++-------------
fs/smb/server/smbacl.h | 2 +-
3 files changed, 29 insertions(+), 16 deletions(-)
diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c
index 230bea45de..8e47d725c1 100644
--- a/fs/smb/server/smb2pdu.c
+++ b/fs/smb/server/smb2pdu.c
@@ -3661,7 +3661,7 @@ static int smb2_create_sd_buffer(struct ksmbd_work *work,
sizeof(struct create_sd_buf_req))
return -EINVAL;
return set_info_sec(work->conn, work->tcon, path, &sd_buf->ntsd,
- le32_to_cpu(sd_buf->ccontext.DataLength), true, false);
+ le32_to_cpu(sd_buf->ccontext.DataLength), false);
}
static void ksmbd_acls_fattr(struct smb_fattr *fattr,
@@ -8535,7 +8535,7 @@ static int smb2_set_info_sec(struct ksmbd_file *fp, int addition_info,
return -EACCES;
return set_info_sec(fp->conn, fp->tcon, &fp->filp->f_path, pntsd,
- buf_len, false, true);
+ buf_len, true);
}
/**
diff --git a/fs/smb/server/smbacl.c b/fs/smb/server/smbacl.c
index 126264cc07..d1cf63a9fe 100644
--- a/fs/smb/server/smbacl.c
+++ b/fs/smb/server/smbacl.c
@@ -1017,12 +1017,14 @@ int parse_sec_desc(struct mnt_idmap *idmap, struct smb_ntsd *pntsd,
le32_to_cpu(pntsd->sacloffset), dacloffset);
pntsd_type = le16_to_cpu(pntsd->type);
- if (!(pntsd_type & DACL_PRESENT)) {
+ /*
+ * Without a DACL only the owner and group change (Windows sends that
+ * to take ownership): don't return before them.
+ */
+ if (pntsd_type & DACL_PRESENT)
+ pntsd->type = cpu_to_le16(DACL_PRESENT);
+ else
ksmbd_debug(SMB, "DACL_PRESENT in DACL type is not set\n");
- return rc;
- }
-
- pntsd->type = cpu_to_le16(DACL_PRESENT);
if (pntsd->osidoffset) {
if (le32_to_cpu(pntsd->osidoffset) < sizeof(struct smb_ntsd))
@@ -1060,6 +1062,9 @@ int parse_sec_desc(struct mnt_idmap *idmap, struct smb_ntsd *pntsd,
}
}
+ if (!(pntsd_type & DACL_PRESENT))
+ return 0;
+
if ((pntsd_type & (DACL_AUTO_INHERITED | DACL_AUTO_INHERIT_REQ)) ==
(DACL_AUTO_INHERITED | DACL_AUTO_INHERIT_REQ))
pntsd->type |= cpu_to_le16(DACL_AUTO_INHERITED);
@@ -1768,18 +1773,21 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, const struct path *path,
int set_info_sec(struct ksmbd_conn *conn, struct ksmbd_tree_connect *tcon,
const struct path *path, struct smb_ntsd *pntsd, int ntsd_len,
- bool type_check, bool get_write)
+ bool get_write)
{
int rc;
struct smb_fattr fattr = {{0}};
struct inode *inode = d_inode(path->dentry);
struct mnt_idmap *idmap = mnt_idmap(path->mnt);
struct iattr newattrs;
+ bool dacl;
fattr.cf_uid = INVALID_UID;
fattr.cf_gid = INVALID_GID;
fattr.cf_mode = inode->i_mode;
+ /* parse_sec_desc() rewrites type */
+ dacl = le16_to_cpu(pntsd->type) & DACL_PRESENT;
rc = parse_sec_desc(idmap, pntsd, ntsd_len, &fattr);
if (rc)
goto out;
@@ -1793,12 +1801,14 @@ int set_info_sec(struct ksmbd_conn *conn, struct ksmbd_tree_connect *tcon,
newattrs.ia_valid |= ATTR_GID;
newattrs.ia_gid = fattr.cf_gid;
}
- newattrs.ia_valid |= ATTR_MODE;
- newattrs.ia_mode = (inode->i_mode & ~0777) | (fattr.cf_mode & 0777);
-
- ksmbd_vfs_remove_acl_xattrs(idmap, path);
+ /* only the owner and group without a DACL: mode and ACLs stay */
+ if (dacl) {
+ newattrs.ia_valid |= ATTR_MODE;
+ newattrs.ia_mode = (inode->i_mode & ~0777) | (fattr.cf_mode & 0777);
+ ksmbd_vfs_remove_acl_xattrs(idmap, path);
+ }
/* Update posix acls */
- if (IS_ENABLED(CONFIG_FS_POSIX_ACL) && fattr.cf_dacls) {
+ if (IS_ENABLED(CONFIG_FS_POSIX_ACL) && dacl && fattr.cf_dacls) {
rc = set_posix_acl(idmap, path->dentry,
ACL_TYPE_ACCESS, fattr.cf_acls);
if (rc < 0)
@@ -1821,8 +1831,11 @@ int set_info_sec(struct ksmbd_conn *conn, struct ksmbd_tree_connect *tcon,
if (rc)
goto out;
- /* Check it only calling from SD BUFFER context */
- if (type_check && !(le16_to_cpu(pntsd->type) & DACL_PRESENT))
+ /*
+ * The stored descriptor keeps its DACL when only the owner or group
+ * changed: the owner shown is the inode's anyway.
+ */
+ if (!dacl)
goto out;
if (test_share_config_flag(tcon->share_conf, KSMBD_SHARE_FLAG_ACL_XATTR)) {
diff --git a/fs/smb/server/smbacl.h b/fs/smb/server/smbacl.h
index b6a01f0d45..d4c9b679d8 100644
--- a/fs/smb/server/smbacl.h
+++ b/fs/smb/server/smbacl.h
@@ -99,7 +99,7 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, const struct path *path,
bool strict);
int set_info_sec(struct ksmbd_conn *conn, struct ksmbd_tree_connect *tcon,
const struct path *path, struct smb_ntsd *pntsd, int ntsd_len,
- bool type_check, bool get_write);
+ bool get_write);
void id_to_sid(unsigned int cid, uint sidtype, struct smb_sid *ssid);
void ksmbd_uid_to_sid(kuid_t uid, uint sidtype, struct smb_sid *ssid);
void ksmbd_gid_to_sid(kgid_t gid, uint sidtype, struct smb_sid *ssid);
--
2.56.0
^ permalink raw reply [flat|nested] 2+ messages in thread
* [RFC PATCH 1/6] ksmbd: don't ignore the owner and group of a descriptor without a DACL
2026-10-28 6:19 [RFC PATCH 0/6] ksmbd: Windows SIDs for user and group ids Jan Koester
@ 2026-10-28 6:19 ` Jan Koester
0 siblings, 0 replies; 2+ messages in thread
From: Jan Koester @ 2026-10-28 6:19 UTC (permalink / raw)
To: Namjae Jeon, Steve French
Cc: Sergey Senozhatsky, Tom Talpey, ChenXiaoSong, Christian Brauner,
linux-cifs, linux-fsdevel, samba-technical, linux-kernel
parse_sec_desc() returned before looking at the owner and group SIDs
when the descriptor had no DACL_PRESENT, so an SMB2 SET_INFO with only
OWNER_SECURITY_INFORMATION or GROUP_SECURITY_INFORMATION -- what Windows
sends to take ownership, and smbcacls -C/-G -- did nothing and still
reported success.
Map the owner and group in any case and parse the DACL only if there is
one. set_info_sec() then changes the mode and the POSIX ACLs only for a
descriptor with a DACL, and leaves the stored descriptor alone without
one: replacing it would drop its DACL, and the owner shown comes from
the inode anyway. This also covers the SD buffer create context, so its
type_check argument goes away.
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Jan Koester <jan.koester@tuxist.de>
---
fs/smb/server/smb2pdu.c | 4 ++--
fs/smb/server/smbacl.c | 39 ++++++++++++++++++++++++++-------------
fs/smb/server/smbacl.h | 2 +-
3 files changed, 29 insertions(+), 16 deletions(-)
diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c
index 230bea45de..8e47d725c1 100644
--- a/fs/smb/server/smb2pdu.c
+++ b/fs/smb/server/smb2pdu.c
@@ -3661,7 +3661,7 @@ static int smb2_create_sd_buffer(struct ksmbd_work *work,
sizeof(struct create_sd_buf_req))
return -EINVAL;
return set_info_sec(work->conn, work->tcon, path, &sd_buf->ntsd,
- le32_to_cpu(sd_buf->ccontext.DataLength), true, false);
+ le32_to_cpu(sd_buf->ccontext.DataLength), false);
}
static void ksmbd_acls_fattr(struct smb_fattr *fattr,
@@ -8535,7 +8535,7 @@ static int smb2_set_info_sec(struct ksmbd_file *fp, int addition_info,
return -EACCES;
return set_info_sec(fp->conn, fp->tcon, &fp->filp->f_path, pntsd,
- buf_len, false, true);
+ buf_len, true);
}
/**
diff --git a/fs/smb/server/smbacl.c b/fs/smb/server/smbacl.c
index 126264cc07..d1cf63a9fe 100644
--- a/fs/smb/server/smbacl.c
+++ b/fs/smb/server/smbacl.c
@@ -1017,12 +1017,14 @@ int parse_sec_desc(struct mnt_idmap *idmap, struct smb_ntsd *pntsd,
le32_to_cpu(pntsd->sacloffset), dacloffset);
pntsd_type = le16_to_cpu(pntsd->type);
- if (!(pntsd_type & DACL_PRESENT)) {
+ /*
+ * Without a DACL only the owner and group change (Windows sends that
+ * to take ownership): don't return before them.
+ */
+ if (pntsd_type & DACL_PRESENT)
+ pntsd->type = cpu_to_le16(DACL_PRESENT);
+ else
ksmbd_debug(SMB, "DACL_PRESENT in DACL type is not set\n");
- return rc;
- }
-
- pntsd->type = cpu_to_le16(DACL_PRESENT);
if (pntsd->osidoffset) {
if (le32_to_cpu(pntsd->osidoffset) < sizeof(struct smb_ntsd))
@@ -1060,6 +1062,9 @@ int parse_sec_desc(struct mnt_idmap *idmap, struct smb_ntsd *pntsd,
}
}
+ if (!(pntsd_type & DACL_PRESENT))
+ return 0;
+
if ((pntsd_type & (DACL_AUTO_INHERITED | DACL_AUTO_INHERIT_REQ)) ==
(DACL_AUTO_INHERITED | DACL_AUTO_INHERIT_REQ))
pntsd->type |= cpu_to_le16(DACL_AUTO_INHERITED);
@@ -1768,18 +1773,21 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, const struct path *path,
int set_info_sec(struct ksmbd_conn *conn, struct ksmbd_tree_connect *tcon,
const struct path *path, struct smb_ntsd *pntsd, int ntsd_len,
- bool type_check, bool get_write)
+ bool get_write)
{
int rc;
struct smb_fattr fattr = {{0}};
struct inode *inode = d_inode(path->dentry);
struct mnt_idmap *idmap = mnt_idmap(path->mnt);
struct iattr newattrs;
+ bool dacl;
fattr.cf_uid = INVALID_UID;
fattr.cf_gid = INVALID_GID;
fattr.cf_mode = inode->i_mode;
+ /* parse_sec_desc() rewrites type */
+ dacl = le16_to_cpu(pntsd->type) & DACL_PRESENT;
rc = parse_sec_desc(idmap, pntsd, ntsd_len, &fattr);
if (rc)
goto out;
@@ -1793,12 +1801,14 @@ int set_info_sec(struct ksmbd_conn *conn, struct ksmbd_tree_connect *tcon,
newattrs.ia_valid |= ATTR_GID;
newattrs.ia_gid = fattr.cf_gid;
}
- newattrs.ia_valid |= ATTR_MODE;
- newattrs.ia_mode = (inode->i_mode & ~0777) | (fattr.cf_mode & 0777);
-
- ksmbd_vfs_remove_acl_xattrs(idmap, path);
+ /* only the owner and group without a DACL: mode and ACLs stay */
+ if (dacl) {
+ newattrs.ia_valid |= ATTR_MODE;
+ newattrs.ia_mode = (inode->i_mode & ~0777) | (fattr.cf_mode & 0777);
+ ksmbd_vfs_remove_acl_xattrs(idmap, path);
+ }
/* Update posix acls */
- if (IS_ENABLED(CONFIG_FS_POSIX_ACL) && fattr.cf_dacls) {
+ if (IS_ENABLED(CONFIG_FS_POSIX_ACL) && dacl && fattr.cf_dacls) {
rc = set_posix_acl(idmap, path->dentry,
ACL_TYPE_ACCESS, fattr.cf_acls);
if (rc < 0)
@@ -1821,8 +1831,11 @@ int set_info_sec(struct ksmbd_conn *conn, struct ksmbd_tree_connect *tcon,
if (rc)
goto out;
- /* Check it only calling from SD BUFFER context */
- if (type_check && !(le16_to_cpu(pntsd->type) & DACL_PRESENT))
+ /*
+ * The stored descriptor keeps its DACL when only the owner or group
+ * changed: the owner shown is the inode's anyway.
+ */
+ if (!dacl)
goto out;
if (test_share_config_flag(tcon->share_conf, KSMBD_SHARE_FLAG_ACL_XATTR)) {
diff --git a/fs/smb/server/smbacl.h b/fs/smb/server/smbacl.h
index b6a01f0d45..d4c9b679d8 100644
--- a/fs/smb/server/smbacl.h
+++ b/fs/smb/server/smbacl.h
@@ -99,7 +99,7 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, const struct path *path,
bool strict);
int set_info_sec(struct ksmbd_conn *conn, struct ksmbd_tree_connect *tcon,
const struct path *path, struct smb_ntsd *pntsd, int ntsd_len,
- bool type_check, bool get_write);
+ bool get_write);
void id_to_sid(unsigned int cid, uint sidtype, struct smb_sid *ssid);
void ksmbd_uid_to_sid(kuid_t uid, uint sidtype, struct smb_sid *ssid);
void ksmbd_gid_to_sid(kgid_t gid, uint sidtype, struct smb_sid *ssid);
--
2.56.0
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-11-10 9:01 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-10 9:02 [RFC PATCH 1/6] ksmbd: don't ignore the owner and group of a descriptor without a DACL Jan Koester
-- strict thread matches above, loose matches on Subject: below --
2026-10-28 6:19 [RFC PATCH 0/6] ksmbd: Windows SIDs for user and group ids Jan Koester
2026-10-28 6:19 ` [RFC PATCH 1/6] ksmbd: don't ignore the owner and group of a descriptor without a DACL Jan Koester
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®