Skip to content

Commit f1abcb4

Browse files
verivusainamjaejeon
authored andcommitted
ksmbd: fix memory leaks and NULL deref in smb2_lock()
smb2_lock() has three error handling issues after list_del() detaches smb_lock from lock_list at no_check_cl: 1) If vfs_lock_file() returns an unexpected error in the non-UNLOCK path, goto out leaks smb_lock and its flock because the out: handler only iterates lock_list and rollback_list, neither of which contains the detached smb_lock. 2) If vfs_lock_file() returns -ENOENT in the UNLOCK path, goto out leaks smb_lock and flock for the same reason. The error code returned to the dispatcher is also stale. 3) In the rollback path, smb_flock_init() can return NULL on allocation failure. The result is dereferenced unconditionally, causing a kernel NULL pointer dereference. Add a NULL check to prevent the crash and clean up the bookkeeping; the VFS lock itself cannot be rolled back without the allocation and will be released at file or connection teardown. Fix cases 1 and 2 by hoisting the locks_free_lock()/kfree() to before the if(!rc) check in the UNLOCK branch so all exit paths share one free site, and by freeing smb_lock and flock before goto out in the non-UNLOCK branch. Propagate the correct error code in both cases. Fix case 3 by wrapping the VFS unlock in an if(rlock) guard and adding a NULL check for locks_free_lock(rlock) in the shared cleanup. Found via call-graph analysis using sqry. Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3") Cc: stable@vger.kernel.org Suggested-by: ChenXiaoSong <chenxiaosong@kylinos.cn> Signed-off-by: Werner Kasselman <werner@verivus.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
1 parent c89936e commit f1abcb4

1 file changed

Lines changed: 19 additions & 10 deletions

File tree

smb2pdu.c

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8197,14 +8197,15 @@ int smb2_lock(struct ksmbd_work *work)
81978197
rc = vfs_lock_file(filp, smb_lock->cmd, flock, NULL);
81988198
skip:
81998199
if (smb_lock->flags & SMB2_LOCKFLAG_UNLOCK) {
8200+
locks_free_lock(flock);
8201+
kfree(smb_lock);
82008202
if (!rc) {
82018203
ksmbd_debug(SMB, "File unlocked\n");
82028204
} else if (rc == -ENOENT) {
82038205
rsp->hdr.Status = STATUS_NOT_LOCKED;
8206+
err = rc;
82048207
goto out;
82058208
}
8206-
locks_free_lock(flock);
8207-
kfree(smb_lock);
82088209
} else {
82098210
if (rc == FILE_LOCK_DEFERRED) {
82108211
void **argv;
@@ -8273,6 +8274,9 @@ int smb2_lock(struct ksmbd_work *work)
82738274
spin_unlock(&work->conn->llist_lock);
82748275
ksmbd_debug(SMB, "successful in taking lock\n");
82758276
} else {
8277+
locks_free_lock(flock);
8278+
kfree(smb_lock);
8279+
err = rc;
82768280
goto out;
82778281
}
82788282
}
@@ -8303,17 +8307,21 @@ int smb2_lock(struct ksmbd_work *work)
83038307
struct file_lock *rlock = NULL;
83048308

83058309
rlock = smb_flock_init(filp);
8310+
if (rlock) {
83068311
#if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 9, 0)
8307-
rlock->c.flc_type = F_UNLCK;
8312+
rlock->c.flc_type = F_UNLCK;
83088313
#else
8309-
rlock->fl_type = F_UNLCK;
8314+
rlock->fl_type = F_UNLCK;
83108315
#endif
8311-
rlock->fl_start = smb_lock->start;
8312-
rlock->fl_end = smb_lock->end;
8316+
rlock->fl_start = smb_lock->start;
8317+
rlock->fl_end = smb_lock->end;
83138318

8314-
rc = vfs_lock_file(filp, F_SETLK, rlock, NULL);
8315-
if (rc)
8316-
pr_err("rollback unlock fail : %d\n", rc);
8319+
rc = vfs_lock_file(filp, F_SETLK, rlock, NULL);
8320+
if (rc)
8321+
pr_err("rollback unlock fail : %d\n", rc);
8322+
} else {
8323+
pr_err("rollback unlock alloc failed\n");
8324+
}
83178325

83188326
list_del(&smb_lock->llist);
83198327
spin_lock(&work->conn->llist_lock);
@@ -8323,7 +8331,8 @@ int smb2_lock(struct ksmbd_work *work)
83238331
spin_unlock(&work->conn->llist_lock);
83248332

83258333
locks_free_lock(smb_lock->fl);
8326-
locks_free_lock(rlock);
8334+
if (rlock)
8335+
locks_free_lock(rlock);
83278336
kfree(smb_lock);
83288337
}
83298338
out2:

0 commit comments

Comments
 (0)