| Message ID | 20230201081010.17446-1-hbh25y@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | ksmbd: fix possible memory leak in smb2_lock() | expand |
2023-02-01 17:10 GMT+09:00, Hangyu Hua <hbh25y@gmail.com>: > argv needs to be free when setup_async_work fails or when the current > process is woken up. > > Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3") > Signed-off-by: Hangyu Hua <hbh25y@gmail.com> > --- > fs/ksmbd/smb2pdu.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c > index d681f91947d9..5b7668c04f76 100644 > --- a/fs/ksmbd/smb2pdu.c > +++ b/fs/ksmbd/smb2pdu.c > @@ -7050,6 +7050,7 @@ int smb2_lock(struct ksmbd_work *work) > smb2_remove_blocked_lock, > argv); > if (rc) { > + kfree(argv); > err = -ENOMEM; > goto out; > } > @@ -7061,6 +7062,8 @@ int smb2_lock(struct ksmbd_work *work) > > ksmbd_vfs_posix_lock_wait(flock); > > + work->cancel_fn = NULL; > + kfree(argv); This change seems to causes a NULL pointer dereference issue in set_close_state_blocked_works(). It is right to free it and set NULL after removing entry from list. > if (work->state != KSMBD_WORK_ACTIVE) { > list_del(&smb_lock->llist); > spin_lock(&work->conn->llist_lock); > -- > 2.34.1 > >
On 4/2/2023 18:10, Namjae Jeon wrote: > 2023-02-01 17:10 GMT+09:00, Hangyu Hua <hbh25y@gmail.com>: >> argv needs to be free when setup_async_work fails or when the current >> process is woken up. >> >> Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3") >> Signed-off-by: Hangyu Hua <hbh25y@gmail.com> >> --- >> fs/ksmbd/smb2pdu.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c >> index d681f91947d9..5b7668c04f76 100644 >> --- a/fs/ksmbd/smb2pdu.c >> +++ b/fs/ksmbd/smb2pdu.c >> @@ -7050,6 +7050,7 @@ int smb2_lock(struct ksmbd_work *work) >> smb2_remove_blocked_lock, >> argv); >> if (rc) { >> + kfree(argv); >> err = -ENOMEM; >> goto out; >> } >> @@ -7061,6 +7062,8 @@ int smb2_lock(struct ksmbd_work *work) >> >> ksmbd_vfs_posix_lock_wait(flock); >> >> + work->cancel_fn = NULL; >> + kfree(argv); > This change seems to causes a NULL pointer dereference issue in > set_close_state_blocked_works(). It is right to free it and set NULL > after removing entry from list. I get it. I will send a v2. Thanks, Hangyu >> if (work->state != KSMBD_WORK_ACTIVE) { >> list_del(&smb_lock->llist); >> spin_lock(&work->conn->llist_lock); >> -- >> 2.34.1 >> >>
diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c index d681f91947d9..5b7668c04f76 100644 --- a/fs/ksmbd/smb2pdu.c +++ b/fs/ksmbd/smb2pdu.c @@ -7050,6 +7050,7 @@ int smb2_lock(struct ksmbd_work *work) smb2_remove_blocked_lock, argv); if (rc) { + kfree(argv); err = -ENOMEM; goto out; } @@ -7061,6 +7062,8 @@ int smb2_lock(struct ksmbd_work *work) ksmbd_vfs_posix_lock_wait(flock); + work->cancel_fn = NULL; + kfree(argv); if (work->state != KSMBD_WORK_ACTIVE) { list_del(&smb_lock->llist); spin_lock(&work->conn->llist_lock);
argv needs to be free when setup_async_work fails or when the current process is woken up. Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3") Signed-off-by: Hangyu Hua <hbh25y@gmail.com> --- fs/ksmbd/smb2pdu.c | 3 +++ 1 file changed, 3 insertions(+)