[PATCH] ksmbd: enforce the DACL permission check on delete-on-close opens

[PATCH] ksmbd: enforce the DACL permission check on delete-on-close opens

[Ian Bridges]

smb2_open() runs the NT ACL permission check, smb_check_perm_dacl(), for
an existing file only when FILE_DELETE_ON_CLOSE is not requested.

On an acl_xattr share the stored security descriptor is the authoritative
permission model. set_info_sec() maps a descriptor into POSIX ACLs and
mode, so its read and write restrictions are also checked by
inode_permission() when smb2_open() opens the file. A file's DELETE right
cannot be expressed in a POSIX ACL, because Unix delete is governed by the
parent directory's write permission and not by the file's own ACL. So
only smb_check_perm_dacl() can check it.

Because delete-on-close opens skip that call, a client can open an
existing file with DesiredAccess FILE_READ_ATTRIBUTES | DELETE and
CreateOptions FILE_DELETE_ON_CLOSE and have the file unlinked on close,
even when the descriptor grants the caller no access at all. Only the
POSIX checks remain, read on the file and write on its parent directory.
The normal open-for-DELETE path is denied by smb_check_perm_dacl(), so
this lets a user delete files that a normal delete request could not.

Run the DACL check for every existing-file open, so a delete-on-close
open gets the same DACL check as any other open of an existing file. A
caller the descriptor does not grant access to is denied. An authorized
caller is unaffected, and shares without a stored descriptor are
unchanged.

Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")
Cc: stable@vger.kernel.org
Signed-off-by: Ian Bridges <icb@fastmail.org>
---

Notes:
    Reachable on a share with "vfs objects = acl_xattr" by an authenticated
    user who passes the POSIX checks for read on the file and write on the
    parent directory, for example a shared drop-box directory where many
    users can write the parent and individual files carry restrictive ACLs.
    Impact is unauthorized file deletion. It is not a privilege escalation
    and does not expose file contents.
    
    Reproduced on ksmbd-for-next 206fc30f71b0. tester2 creates a file and
    sets a DACL granting only tester2. tester's open(DELETE) fails with
    STATUS_ACCESS_DENIED, but open(FILE_READ_ATTRIBUTES | DELETE) with
    FILE_DELETE_ON_CLOSE succeeds and the file is unlinked on close. With
    this change that open returns STATUS_ACCESS_DENIED and the file
    survives. A file's owner can still delete it with delete-on-close when
    the DACL grants them access, and shares without a stored descriptor are
    unaffected. A reproducer is available on request.
    
    This fix was developed with LLM assistance.

 fs/smb/server/smb2pdu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c
index be226664b8af..b664a02057eb 100644
--- a/fs/smb/server/smb2pdu.c
+++ b/fs/smb/server/smb2pdu.c
@@ -4836,7 +4836,7 @@ int smb2_open(struct ksmbd_work *work)
 
 	daccess = smb_map_generic_desired_access(req->DesiredAccess);
 
-	if (file_present && !(req->CreateOptions & FILE_DELETE_ON_CLOSE_LE)) {
+	if (file_present) {
 		rc = smb_check_perm_dacl(conn, &path, &daccess,
 					 req->DesiredAccess,
 					 sess->user->uid, false);

base-commit: 206fc30f71b005156085fd8523ccc84a2c67791b
-- 
2.47.3

Re: [PATCH] ksmbd: enforce the DACL permission check on delete-on-close opens

[Namjae Jeon]

On Thu, Oct 29, 2026 at 12:35 AM Ian Bridges <icb@fastmail.org> wrote:
>
> smb2_open() runs the NT ACL permission check, smb_check_perm_dacl(), for
> an existing file only when FILE_DELETE_ON_CLOSE is not requested.
>
> On an acl_xattr share the stored security descriptor is the authoritative
> permission model. set_info_sec() maps a descriptor into POSIX ACLs and
> mode, so its read and write restrictions are also checked by
> inode_permission() when smb2_open() opens the file. A file's DELETE right
> cannot be expressed in a POSIX ACL, because Unix delete is governed by the
> parent directory's write permission and not by the file's own ACL. So
> only smb_check_perm_dacl() can check it.
>
> Because delete-on-close opens skip that call, a client can open an
> existing file with DesiredAccess FILE_READ_ATTRIBUTES | DELETE and
> CreateOptions FILE_DELETE_ON_CLOSE and have the file unlinked on close,
> even when the descriptor grants the caller no access at all. Only the
> POSIX checks remain, read on the file and write on its parent directory.
> The normal open-for-DELETE path is denied by smb_check_perm_dacl(), so
> this lets a user delete files that a normal delete request could not.
>
> Run the DACL check for every existing-file open, so a delete-on-close
> open gets the same DACL check as any other open of an existing file. A
> caller the descriptor does not grant access to is denied. An authorized
> caller is unaffected, and shares without a stored descriptor are
> unchanged.
>
> Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")
> Cc: stable@vger.kernel.org
> Signed-off-by: Ian Bridges <icb@fastmail.org>
This patch causes smb2.acls.INHERITANCE failure in smbtorture test.
Can you check it ?

failure: INHERITANCE [
(../../source4/torture/smb2/acls.c:1054) Incorrect status
NT_STATUS_OBJECT_NAME_COLLISION - should be NT_STATUS_OK
]