Thanks for your reply. I will send v2 patchset soon based on your
suggestions.
在 2024/8/22 08:41, Namjae Jeon 写道:
On Tue, Aug 20, 2024 at 11:35 PM <chenxiaosong@xxxxxxxxxxxxxxxx> wrote:
From: ChenXiaoSong <chenxiaosong@xxxxxxxxxx>
null-ptr-deref will occur when (req_op_level == SMB2_OPLOCK_LEVEL_LEASE)
and parse_lease_state() return NULL.
Fix this by returning error pointer on parse_lease_state() and checking
error.
Signed-off-by: ChenXiaoSong <chenxiaosong@xxxxxxxxxxxxxxxx>
We intended for it to return null. We shouldn't handle the error even
if it fails.
All places check if lc is null except the one.
We can fix it like the following one. please send this patch if you are okay.
diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c
index b6c5a8ea3887..884e21992c92 100644
--- a/fs/smb/server/smb2pdu.c
+++ b/fs/smb/server/smb2pdu.c
@@ -3404,7 +3404,7 @@ int smb2_open(struct ksmbd_work *work)
goto err_out1;
}
} else {
- if (req_op_level == SMB2_OPLOCK_LEVEL_LEASE) {
+ if (req_op_level == SMB2_OPLOCK_LEVEL_LEASE && lc) {
if (S_ISDIR(file_inode(filp)->i_mode)) {
lc->req_state &= ~SMB2_LEASE_WRITE_CACHING_LE;
lc->is_dir = true;
---
fs/smb/server/oplock.c | 11 +++++++----
fs/smb/server/smb2pdu.c | 17 ++++++++++++-----
2 files changed, 19 insertions(+), 9 deletions(-)
diff --git a/fs/smb/server/oplock.c b/fs/smb/server/oplock.c
index a8f52c4ebbda..e8591686a037 100644
--- a/fs/smb/server/oplock.c
+++ b/fs/smb/server/oplock.c
@@ -1510,7 +1510,8 @@ void create_lease_buf(u8 *rbuf, struct lease *lease)
* parse_lease_state() - parse lease context containted in file open request
* @open_req: buffer containing smb2 file open(create) request
*
- * Return: oplock state, -ENOENT if create lease context not found
+ * Return: allocated lease context object on success, otherwise error pointer.
+ * -ENOENT pointer if create lease context not found.
*/
struct lease_ctx_info *parse_lease_state(void *open_req)
{
@@ -1519,12 +1520,14 @@ struct lease_ctx_info *parse_lease_state(void *open_req)
struct lease_ctx_info *lreq;
cc = smb2_find_context_vals(req, SMB2_CREATE_REQUEST_LEASE, 4);
- if (IS_ERR_OR_NULL(cc))
- return NULL;
+ if (!cc)
+ return ERR_PTR(-ENOENT);
+ if (IS_ERR(cc))
+ return ERR_CAST(cc);
lreq = kzalloc(sizeof(struct lease_ctx_info), GFP_KERNEL);
if (!lreq)
- return NULL;
+ return ERR_PTR(-ENOMEM);
if (sizeof(struct lease_context_v2) == le32_to_cpu(cc->DataLength)) {
struct create_lease_v2 *lc = (struct create_lease_v2 *)cc;
diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c
index d8a827e0dced..119c1ba5f255 100644
--- a/fs/smb/server/smb2pdu.c
+++ b/fs/smb/server/smb2pdu.c
@@ -2767,8 +2767,9 @@ static int parse_durable_handle_context(struct ksmbd_work *work,
}
}
- if (((lc && (lc->req_state & SMB2_LEASE_HANDLE_CACHING_LE)) ||
- req_op_level == SMB2_OPLOCK_LEVEL_BATCH)) {
+ if ((!IS_ERR_OR_NULL(lc) > 0 &&
+ (lc->req_state & SMB2_LEASE_HANDLE_CACHING_LE)) ||
+ req_op_level == SMB2_OPLOCK_LEVEL_BATCH) {
dh_info->CreateGuid =
durable_v2_blob->CreateGuid;
dh_info->persistent =
@@ -2788,8 +2789,9 @@ static int parse_durable_handle_context(struct ksmbd_work *work,
goto out;
}
- if (((lc && (lc->req_state & SMB2_LEASE_HANDLE_CACHING_LE)) ||
- req_op_level == SMB2_OPLOCK_LEVEL_BATCH)) {
+ if ((!IS_ERR_OR_NULL(lc) &&
+ (lc->req_state & SMB2_LEASE_HANDLE_CACHING_LE)) ||
+ req_op_level == SMB2_OPLOCK_LEVEL_BATCH) {
ksmbd_debug(SMB, "Request for durable open\n");
dh_info->type = dh_idx;
}
@@ -2935,8 +2937,13 @@ int smb2_open(struct ksmbd_work *work)
ksmbd_put_durable_fd(fp);
goto reconnected_fp;
}
- } else if (req_op_level == SMB2_OPLOCK_LEVEL_LEASE)
+ } else if (req_op_level == SMB2_OPLOCK_LEVEL_LEASE) {
lc = parse_lease_state(req);
+ if (IS_ERR(lc)) {
+ rc = PTR_ERR(lc);
+ goto err_out2;
+ }
+ }
if (le32_to_cpu(req->ImpersonationLevel) > le32_to_cpu(IL_DELEGATE)) {
pr_err("Invalid impersonationlevel : 0x%x\n",
--
2.34.1
