Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

KSMBD_DCERPC_RETURN_READY should be cleared if the return isn't ready #199

Open
rtmrtmrtmrtm opened this issue Nov 14, 2023 · 3 comments
Open

Comments

@rtmrtmrtmrtm
Copy link

rpc_write_request() always sets

dce->flags |= KSMBD_DCERPC_RETURN_READY;

even if it returns failure. In that case, the flag isn't cleared,
since that would be done in rpc_read_request(), which isn't called on
rpc_write_request() failure. So the next RPC to arrive does nothing in
rpc_write_request():

if (pipe->dce->flags & KSMBD_DCERPC_RETURN_READY)
	return KSMBD_RPC_OK;

But the KSMBD_RPC_OK causes rpc_ioctl_request() to assume the results
from the previous failed rpc_write_request() are valid, and it calls
rpc_read_request(). This might cause crashes from use of unexpectedly
NULL or partially initialized pipe->dce->* request data.

I'm submitting this as an issue rather than a patch because I'm not
sure what the flag implies, or how to clean up pipe->dce->* upon
rpc_write_request() failure. Perhaps rpc_ioctl_request() should clear
the RETURN_READY flag on failure, and perhaps rpc_request() should do
the same for RPC_WRITE_METHOD.

@namjaejeon
Copy link
Owner

We can clear KSMBD_DCERPC_RETURN_READY flags in rpc_write_request() on error. Normally, client send KSMBD_RPC_CLOSE_METHOD request(pipe will be freed) if KSMBD_RPC_IOCTL_METHOD request fail. and then KSMBD_RPC_OPEN_METHOD request(pipe is newly allocated) will come again. So this problem was not actually discovered. Did you find this with TC? and you can send the patch for this.

@rtmrtmrtmrtm
Copy link
Author

What is TC?

@namjaejeon
Copy link
Owner

Ah.. TC is testcase... You sent me a testcase to reproduce kernel oops from ksmbd kernel module before. So I thought you had the testcase to make crash from ksmbd-mountd.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants