Skip to content

Commit 48623ec

Browse files
verivusaismfrench
authored andcommitted
ksmbd: fix use-after-free and NULL deref in smb_grant_oplock()
smb_grant_oplock() has two issues in the oplock publication sequence: 1) opinfo is linked into ci->m_op_list (via opinfo_add) before add_lease_global_list() is called. If add_lease_global_list() fails (kmalloc returns NULL), the error path frees the opinfo via __free_opinfo() while it is still linked in ci->m_op_list. Concurrent m_op_list readers (opinfo_get_list, or direct iteration in smb_break_all_levII_oplock) dereference the freed node. 2) opinfo->o_fp is assigned after add_lease_global_list() publishes the opinfo on the global lease list. A concurrent find_same_lease_key() can walk the lease list and dereference opinfo->o_fp->f_ci while o_fp is still NULL. Fix by restructuring the publication sequence to eliminate post-publish failure: - Set opinfo->o_fp before any list publication (fixes NULL deref). - Preallocate lease_table via alloc_lease_table() before opinfo_add() so add_lease_global_list() becomes infallible after publication. - Keep the original m_op_list publication order (opinfo_add before lease list) so concurrent opens via same_client_has_lease() and opinfo_get_list() still see the in-flight grant. - Use opinfo_put() instead of __free_opinfo() on err_out so that the RCU-deferred free path is used. This also requires splitting add_lease_global_list() to take a preallocated lease_table and changing its return type from int to void, since it can no longer fail. Fixes: 1dfd062 ("ksmbd: fix use-after-free by using call_rcu() for oplock_info") Cc: [email protected] Signed-off-by: Werner Kasselman <[email protected]> Reviewed-by: ChenXiaoSong <[email protected]> Acked-by: Namjae Jeon <[email protected]> Signed-off-by: Steve French <[email protected]>
1 parent 9bbb19d commit 48623ec

1 file changed

Lines changed: 45 additions & 27 deletions

File tree

fs/smb/server/oplock.c

Lines changed: 45 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -82,11 +82,19 @@ static void lease_del_list(struct oplock_info *opinfo)
8282
spin_unlock(&lb->lb_lock);
8383
}
8484

85-
static void lb_add(struct lease_table *lb)
85+
static struct lease_table *alloc_lease_table(struct oplock_info *opinfo)
8686
{
87-
write_lock(&lease_list_lock);
88-
list_add(&lb->l_entry, &lease_table_list);
89-
write_unlock(&lease_list_lock);
87+
struct lease_table *lb;
88+
89+
lb = kmalloc_obj(struct lease_table, KSMBD_DEFAULT_GFP);
90+
if (!lb)
91+
return NULL;
92+
93+
memcpy(lb->client_guid, opinfo->conn->ClientGUID,
94+
SMB2_CLIENT_GUID_SIZE);
95+
INIT_LIST_HEAD(&lb->lease_list);
96+
spin_lock_init(&lb->lb_lock);
97+
return lb;
9098
}
9199

92100
static int alloc_lease(struct oplock_info *opinfo, struct lease_ctx_info *lctx)
@@ -1042,34 +1050,27 @@ static void copy_lease(struct oplock_info *op1, struct oplock_info *op2)
10421050
lease2->version = lease1->version;
10431051
}
10441052

1045-
static int add_lease_global_list(struct oplock_info *opinfo)
1053+
static void add_lease_global_list(struct oplock_info *opinfo,
1054+
struct lease_table *new_lb)
10461055
{
10471056
struct lease_table *lb;
10481057

1049-
read_lock(&lease_list_lock);
1058+
write_lock(&lease_list_lock);
10501059
list_for_each_entry(lb, &lease_table_list, l_entry) {
10511060
if (!memcmp(lb->client_guid, opinfo->conn->ClientGUID,
10521061
SMB2_CLIENT_GUID_SIZE)) {
10531062
opinfo->o_lease->l_lb = lb;
10541063
lease_add_list(opinfo);
1055-
read_unlock(&lease_list_lock);
1056-
return 0;
1064+
write_unlock(&lease_list_lock);
1065+
kfree(new_lb);
1066+
return;
10571067
}
10581068
}
1059-
read_unlock(&lease_list_lock);
10601069

1061-
lb = kmalloc_obj(struct lease_table, KSMBD_DEFAULT_GFP);
1062-
if (!lb)
1063-
return -ENOMEM;
1064-
1065-
memcpy(lb->client_guid, opinfo->conn->ClientGUID,
1066-
SMB2_CLIENT_GUID_SIZE);
1067-
INIT_LIST_HEAD(&lb->lease_list);
1068-
spin_lock_init(&lb->lb_lock);
1069-
opinfo->o_lease->l_lb = lb;
1070+
opinfo->o_lease->l_lb = new_lb;
10701071
lease_add_list(opinfo);
1071-
lb_add(lb);
1072-
return 0;
1072+
list_add(&new_lb->l_entry, &lease_table_list);
1073+
write_unlock(&lease_list_lock);
10731074
}
10741075

10751076
static void set_oplock_level(struct oplock_info *opinfo, int level,
@@ -1189,6 +1190,7 @@ int smb_grant_oplock(struct ksmbd_work *work, int req_op_level, u64 pid,
11891190
int err = 0;
11901191
struct oplock_info *opinfo = NULL, *prev_opinfo = NULL;
11911192
struct ksmbd_inode *ci = fp->f_ci;
1193+
struct lease_table *new_lb = NULL;
11921194
bool prev_op_has_lease;
11931195
__le32 prev_op_state = 0;
11941196

@@ -1291,21 +1293,37 @@ int smb_grant_oplock(struct ksmbd_work *work, int req_op_level, u64 pid,
12911293
set_oplock_level(opinfo, req_op_level, lctx);
12921294

12931295
out:
1294-
opinfo_count_inc(fp);
1295-
opinfo_add(opinfo, fp);
1296-
1296+
/*
1297+
* Set o_fp before any publication so that concurrent readers
1298+
* (e.g. find_same_lease_key() on the lease list) that
1299+
* dereference opinfo->o_fp don't hit a NULL pointer.
1300+
*
1301+
* Keep the original publication order so concurrent opens can
1302+
* still observe the in-flight grant via ci->m_op_list, but make
1303+
* everything after opinfo_add() no-fail by preallocating any new
1304+
* lease_table first.
1305+
*/
1306+
opinfo->o_fp = fp;
12971307
if (opinfo->is_lease) {
1298-
err = add_lease_global_list(opinfo);
1299-
if (err)
1308+
new_lb = alloc_lease_table(opinfo);
1309+
if (!new_lb) {
1310+
err = -ENOMEM;
13001311
goto err_out;
1312+
}
13011313
}
13021314

1315+
opinfo_count_inc(fp);
1316+
opinfo_add(opinfo, fp);
1317+
1318+
if (opinfo->is_lease)
1319+
add_lease_global_list(opinfo, new_lb);
1320+
13031321
rcu_assign_pointer(fp->f_opinfo, opinfo);
1304-
opinfo->o_fp = fp;
13051322

13061323
return 0;
13071324
err_out:
1308-
__free_opinfo(opinfo);
1325+
kfree(new_lb);
1326+
opinfo_put(opinfo);
13091327
return err;
13101328
}
13111329

0 commit comments

Comments
 (0)