ksmbd: fix use-after-free of fp->owner.name in durable handle owner check

Two concurrent SMB2 durable reconnects (DH2C/DHnC) on the same
persistent_id race the fp->owner.name compare-read in
ksmbd_vfs_compare_durable_owner() against the kfree() in
ksmbd_reopen_durable_fd()'s reopen-success path. fp->owner.name is a
standalone kstrdup() buffer whose lifetime is independent of the fp
refcount, and the two sites share no lock: the compare reads the buffer
while the reopen frees it, so the strcmp() can dereference freed memory.

Commit 7ce4fc4001 ("ksmbd: fix durable reconnect double-bind race in
ksmbd_reopen_durable_fd") made the fp->conn claim atomic under
global_ft.lock (closing the owner.name double-free and the ksmbd_file
write-UAF), but the compare-read versus reopen-free pair was left
unserialized.

  BUG: KASAN: slab-use-after-free in strcmp+0x2c/0x80
  Read of size 1 by task kworker
    strcmp
    ksmbd_vfs_compare_durable_owner
    smb2_check_durable_oplock
    smb2_open
  Freed by task kworker:
    kfree
    ksmbd_reopen_durable_fd
    smb2_open
  Allocated by task kworker:
    kstrdup
    session_fd_check
    smb2_session_logoff
  The buggy address belongs to the cache kmalloc-8

Serialize both sides of the race with fp->f_lock.  The global durable
file-table lock still protects the durable reconnect claim, but
fp->owner.name is per-open state and does not need to block unrelated
durable table lookups or reconnects.  The teardown is left at its
existing location after the reopen-success point so that an __open_id()
rollback still retains owner.name for a later legitimate reconnect to
verify.

Fixes: 49110a8ce6 ("ksmbd: validate owner of durable handle on reconnect")
Assisted-by: Henry (Claude):claude-opus-4
Signed-off-by: Gil Portnoy <dddhkts1@gmail.com>
Co-developed-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
This commit is contained in:
Gil Portnoy 2026-06-26 20:38:20 +03:00 committed by Steve French
parent 851ed9e096
commit 3863716350

View File

@ -1461,16 +1461,21 @@ void ksmbd_stop_durable_scavenger(void)
static int ksmbd_vfs_copy_durable_owner(struct ksmbd_file *fp,
struct ksmbd_user *user)
{
char *name;
if (!user)
return -EINVAL;
/* Duplicate the user name to ensure identity persistence */
fp->owner.name = kstrdup(user->name, GFP_KERNEL);
if (!fp->owner.name)
name = kstrdup(user->name, GFP_KERNEL);
if (!name)
return -ENOMEM;
spin_lock(&fp->f_lock);
fp->owner.uid = user->uid;
fp->owner.gid = user->gid;
fp->owner.name = name;
spin_unlock(&fp->f_lock);
return 0;
}
@ -1488,18 +1493,24 @@ static int ksmbd_vfs_copy_durable_owner(struct ksmbd_file *fp,
bool ksmbd_vfs_compare_durable_owner(struct ksmbd_file *fp,
struct ksmbd_user *user)
{
if (!user || !fp->owner.name)
bool ret = false;
if (!user)
return false;
spin_lock(&fp->f_lock);
if (!fp->owner.name)
goto out;
/* Check if the UID and GID match first (fast path) */
if (fp->owner.uid != user->uid || fp->owner.gid != user->gid)
return false;
goto out;
/* Validate the account name to ensure the same SecurityContext */
if (strcmp(fp->owner.name, user->name))
return false;
return true;
ret = (strcmp(fp->owner.name, user->name) == 0);
out:
spin_unlock(&fp->f_lock);
return ret;
}
static bool session_fd_check(struct ksmbd_tree_connect *tcon,
@ -1694,9 +1705,11 @@ int ksmbd_reopen_durable_fd(struct ksmbd_work *work, struct ksmbd_file *fp)
}
up_write(&ci->m_lock);
spin_lock(&fp->f_lock);
fp->owner.uid = fp->owner.gid = 0;
kfree(fp->owner.name);
fp->owner.name = NULL;
spin_unlock(&fp->f_lock);
return 0;
}