Skip to content

Commit

Permalink
ksmbd: fix UAF issue from opinfo->conn
Browse files Browse the repository at this point in the history 
If opinfo->conn is another connection and while ksmbd send oplock break
request to cient on current connection, The connection for opinfo->conn
can be disconnect and conn could be freed. When sending oplock break
request, this ksmbd_conn can be used and cause user-after-free issue.
When getting opinfo from the list, ksmbd check connection is being
released. If it is not released, Increase ->r_count to wait that connection
is freed.

Reported-by: Per Forlin <[email protected]>
Tested-by: Per Forlin <[email protected]>
Signed-off-by: Namjae Jeon <[email protected]>
  • Loading branch information
@namjaejeon
namjaejeon committed May 17, 2023
1 parent 8bd0ddf commit 11aa06f
Showing 1 changed file with 47 additions and 33 deletions.
80 changes: 47 additions & 33 deletions oplock.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -163,13 +163,42 @@ static struct oplock_info *opinfo_get_list(struct ksmbd_inode *ci)
rcu_read_lock();
opinfo = list_first_or_null_rcu(&ci->m_op_list, struct oplock_info,
op_entry);
if (opinfo && !atomic_inc_not_zero(&opinfo->refcount))
opinfo = NULL;
if (opinfo) {
if (!atomic_inc_not_zero(&opinfo->refcount))
opinfo = NULL;
else {
atomic_inc(&opinfo->conn->r_count);
if (ksmbd_conn_releasing(opinfo->conn)) {
atomic_dec(&opinfo->conn->r_count);
atomic_dec(&opinfo->refcount);
opinfo = NULL;
}
}
}

rcu_read_unlock();

return opinfo;
}

static void opinfo_conn_put(struct oplock_info *opinfo)
{
struct ksmbd_conn *conn;

if (!opinfo)
return;

conn = opinfo->conn;
/*
* Checking waitqueue to dropping pending requests on
* disconnection. waitqueue_active is safe because it
* uses atomic operation for condition.
*/
if (!atomic_dec_return(&conn->r_count) && waitqueue_active(&conn->r_count_q))
wake_up(&conn->r_count_q);
opinfo_put(opinfo);
}

void opinfo_put(struct oplock_info *opinfo)
{
if (!atomic_dec_and_test(&opinfo->refcount))
Expand Down Expand Up @@ -763,13 +792,6 @@ static void __smb1_oplock_break_noti(struct work_struct *wk)

ksmbd_conn_write(work);
ksmbd_free_work_struct(work);
/*
* Checking waitqueue to dropping pending requests on
* disconnection. waitqueue_active is safe because it
* uses atomic operation for condition.
*/
if (!atomic_dec_return(&conn->r_count) && waitqueue_active(&conn->r_count_q))
wake_up(&conn->r_count_q);
}

/**
Expand All @@ -791,7 +813,6 @@ static int smb1_oplock_break_noti(struct oplock_info *opinfo)
work->request_buf = (char *)opinfo;
work->conn = conn;

atomic_inc(&conn->r_count);
if (opinfo->op_state == OPLOCK_ACK_WAIT) {
INIT_WORK(&work->work, __smb1_oplock_break_noti);
ksmbd_queue_work(work);
Expand Down Expand Up @@ -876,13 +897,6 @@ static void __smb2_oplock_break_noti(struct work_struct *wk)

out:
ksmbd_free_work_struct(work);
/*
* Checking waitqueue to dropping pending requests on
* disconnection. waitqueue_active is safe because it
* uses atomic operation for condition.
*/
if (!atomic_dec_return(&conn->r_count) && waitqueue_active(&conn->r_count_q))
wake_up(&conn->r_count_q);
}

/**
Expand Down Expand Up @@ -916,7 +930,6 @@ static int smb2_oplock_break_noti(struct oplock_info *opinfo)
work->conn = conn;
work->sess = opinfo->sess;

atomic_inc(&conn->r_count);
if (opinfo->op_state == OPLOCK_ACK_WAIT) {
INIT_WORK(&work->work, __smb2_oplock_break_noti);
ksmbd_queue_work(work);
Expand Down Expand Up @@ -986,13 +999,6 @@ static void __smb2_lease_break_noti(struct work_struct *wk)

out:
ksmbd_free_work_struct(work);
/*
* Checking waitqueue to dropping pending requests on
* disconnection. waitqueue_active is safe because it
* uses atomic operation for condition.
*/
if (!atomic_dec_return(&conn->r_count) && waitqueue_active(&conn->r_count_q))
wake_up(&conn->r_count_q);
}

/**
Expand Down Expand Up @@ -1032,7 +1038,6 @@ static int smb2_lease_break_noti(struct oplock_info *opinfo)
work->conn = conn;
work->sess = opinfo->sess;

atomic_inc(&conn->r_count);
if (opinfo->op_state == OPLOCK_ACK_WAIT) {
list_for_each_safe(tmp, t, &opinfo->interim_list) {
struct ksmbd_work *in_work;
Expand Down Expand Up @@ -1368,28 +1373,30 @@ int smb_grant_oplock(struct ksmbd_work *work, int req_op_level, u64 pid,
}
prev_opinfo = opinfo_get_list(ci);
if (!prev_opinfo ||
(prev_opinfo->level == SMB2_OPLOCK_LEVEL_NONE && lctx))
(prev_opinfo->level == SMB2_OPLOCK_LEVEL_NONE && lctx)) {
opinfo_conn_put(prev_opinfo);
goto set_lev;
}
prev_op_has_lease = prev_opinfo->is_lease;
if (prev_op_has_lease)
prev_op_state = prev_opinfo->o_lease->state;

if (share_ret < 0 &&
prev_opinfo->level == SMB2_OPLOCK_LEVEL_EXCLUSIVE) {
err = share_ret;
opinfo_put(prev_opinfo);
opinfo_conn_put(prev_opinfo);
goto err_out;
}

if (prev_opinfo->level != SMB2_OPLOCK_LEVEL_BATCH &&
prev_opinfo->level != SMB2_OPLOCK_LEVEL_EXCLUSIVE) {
opinfo_put(prev_opinfo);
opinfo_conn_put(prev_opinfo);
goto op_break_not_needed;
}

list_add(&work->interim_entry, &prev_opinfo->interim_list);
err = oplock_break(prev_opinfo, SMB2_OPLOCK_LEVEL_II);
opinfo_put(prev_opinfo);
opinfo_conn_put(prev_opinfo);
if (err == -ENOENT)
goto set_lev;
/* Check all oplock was freed by close */
Expand Down Expand Up @@ -1452,14 +1459,14 @@ static void smb_break_all_write_oplock(struct ksmbd_work *work,
return;
if (brk_opinfo->level != SMB2_OPLOCK_LEVEL_BATCH &&
brk_opinfo->level != SMB2_OPLOCK_LEVEL_EXCLUSIVE) {
opinfo_put(brk_opinfo);
opinfo_conn_put(brk_opinfo);
return;
}

brk_opinfo->open_trunc = is_trunc;
list_add(&work->interim_entry, &brk_opinfo->interim_list);
oplock_break(brk_opinfo, SMB2_OPLOCK_LEVEL_II);
opinfo_put(brk_opinfo);
opinfo_conn_put(brk_opinfo);
}

/**
Expand Down Expand Up @@ -1487,6 +1494,13 @@ void smb_break_all_levII_oplock(struct ksmbd_work *work, struct ksmbd_file *fp,
list_for_each_entry_rcu(brk_op, &ci->m_op_list, op_entry) {
if (!atomic_inc_not_zero(&brk_op->refcount))
continue;

atomic_inc(&brk_op->conn->r_count);
if (ksmbd_conn_releasing(brk_op->conn)) {
atomic_dec(&brk_op->conn->r_count);
continue;
}

rcu_read_unlock();

#ifdef CONFIG_SMB_INSECURE_SERVER
Expand Down Expand Up @@ -1547,7 +1561,7 @@ void smb_break_all_levII_oplock(struct ksmbd_work *work, struct ksmbd_file *fp,
brk_op->open_trunc = is_trunc;
oplock_break(brk_op, SMB2_OPLOCK_LEVEL_NONE);
next:
opinfo_put(brk_op);
opinfo_conn_put(brk_op);
rcu_read_lock();
}
rcu_read_unlock();
Expand Down

0 comments on commit 11aa06f

Please sign in to comment.