Skip to content

Commit 3c21ff8

Browse files
verivusainamjaejeon
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: 1dfd062caa16 ("ksmbd: fix use-after-free by using call_rcu() for oplock_info") Cc: stable@vger.kernel.org Signed-off-by: Werner Kasselman <werner@verivus.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
1 parent 4834238 commit 3c21ff8

1 file changed

Lines changed: 45 additions & 28 deletions

File tree

oplock.c

Lines changed: 45 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -88,11 +88,19 @@ static void lease_del_list(struct oplock_info *opinfo)
8888
spin_unlock(&lb->lb_lock);
8989
}
9090

91-
static void lb_add(struct lease_table *lb)
91+
static struct lease_table *alloc_lease_table(struct oplock_info *opinfo)
9292
{
93-
write_lock(&lease_list_lock);
94-
list_add(&lb->l_entry, &lease_table_list);
95-
write_unlock(&lease_list_lock);
93+
struct lease_table *lb;
94+
95+
lb = kmalloc_obj(struct lease_table, KSMBD_DEFAULT_GFP);
96+
if (!lb)
97+
return NULL;
98+
99+
memcpy(lb->client_guid, opinfo->conn->ClientGUID,
100+
SMB2_CLIENT_GUID_SIZE);
101+
INIT_LIST_HEAD(&lb->lease_list);
102+
spin_lock_init(&lb->lb_lock);
103+
return lb;
96104
}
97105

98106
static int alloc_lease(struct oplock_info *opinfo, struct lease_ctx_info *lctx)
@@ -1256,34 +1264,26 @@ static void copy_lease(struct oplock_info *op1, struct oplock_info *op2)
12561264
lease2->version = lease1->version;
12571265
}
12581266

1259-
static int add_lease_global_list(struct oplock_info *opinfo)
1267+
static int add_lease_global_list(struct oplock_info *opinfo,
1268+
struct lease_table *new_lb)
12601269
{
12611270
struct lease_table *lb;
12621271

1263-
read_lock(&lease_list_lock);
1272+
write_lock(&lease_list_lock);
12641273
list_for_each_entry(lb, &lease_table_list, l_entry) {
12651274
if (!memcmp(lb->client_guid, opinfo->conn->ClientGUID,
12661275
SMB2_CLIENT_GUID_SIZE)) {
12671276
opinfo->o_lease->l_lb = lb;
12681277
lease_add_list(opinfo);
1269-
read_unlock(&lease_list_lock);
1270-
return 0;
1278+
write_unlock(&lease_list_lock);
1279+
kfree(new_lb);
1280+
return;
12711281
}
12721282
}
1273-
read_unlock(&lease_list_lock);
1274-
1275-
lb = kmalloc(sizeof(struct lease_table), KSMBD_DEFAULT_GFP);
1276-
if (!lb)
1277-
return -ENOMEM;
1278-
1279-
memcpy(lb->client_guid, opinfo->conn->ClientGUID,
1280-
SMB2_CLIENT_GUID_SIZE);
1281-
INIT_LIST_HEAD(&lb->lease_list);
1282-
spin_lock_init(&lb->lb_lock);
1283-
opinfo->o_lease->l_lb = lb;
1283+
opinfo->o_lease->l_lb = new_lb;
12841284
lease_add_list(opinfo);
1285-
lb_add(lb);
1286-
return 0;
1285+
list_add(&new_lb->l_entry, &lease_table_list);
1286+
write_unlock(&lease_list_lock);
12871287
}
12881288

12891289
static void set_oplock_level(struct oplock_info *opinfo, int level,
@@ -1407,6 +1407,7 @@ int smb_grant_oplock(struct ksmbd_work *work, int req_op_level, u64 pid,
14071407
int err = 0;
14081408
struct oplock_info *opinfo = NULL, *prev_opinfo = NULL;
14091409
struct ksmbd_inode *ci = fp->f_ci;
1410+
struct lease_table *new_lb = NULL;
14101411
bool prev_op_has_lease;
14111412
__le32 prev_op_state = 0;
14121413

@@ -1509,21 +1510,37 @@ int smb_grant_oplock(struct ksmbd_work *work, int req_op_level, u64 pid,
15091510
set_oplock_level(opinfo, req_op_level, lctx);
15101511

15111512
out:
1512-
opinfo_count_inc(fp);
1513-
opinfo_add(opinfo, fp);
1514-
1513+
/*
1514+
* Set o_fp before any publication so that concurrent readers
1515+
* (e.g. find_same_lease_key() on the lease list) that
1516+
* dereference opinfo->o_fp don't hit a NULL pointer.
1517+
*
1518+
* Keep the original publication order so concurrent opens can
1519+
* still observe the in-flight grant via ci->m_op_list, but make
1520+
* everything after opinfo_add() no-fail by preallocating any new
1521+
* lease_table first.
1522+
*/
1523+
opinfo->o_fp = fp;
15151524
if (opinfo->is_lease) {
1516-
err = add_lease_global_list(opinfo);
1517-
if (err)
1525+
new_lb = alloc_lease_table(opinfo);
1526+
if (!new_lb) {
1527+
err = -ENOMEM;
15181528
goto err_out;
1529+
}
15191530
}
15201531

1532+
opinfo_count_inc(fp);
1533+
opinfo_add(opinfo, fp);
1534+
1535+
if (opinfo->is_lease)
1536+
add_lease_global_list(opinfo, new_lb);
1537+
15211538
rcu_assign_pointer(fp->f_opinfo, opinfo);
1522-
opinfo->o_fp = fp;
15231539

15241540
return 0;
15251541
err_out:
1526-
__free_opinfo(opinfo);
1542+
kfree(new_lb);
1543+
opinfo_put(opinfo);
15271544
return err;
15281545
}
15291546

0 commit comments

Comments
 (0)