namjaejeon / ksmbd-tools

ksmbd kernel server userspace utilities
GNU General Public License v2.0
55 stars 43 forks source link

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

Open rtmrtmrtmrtm opened 12 months ago

rtmrtmrtmrtm commented 12 months ago

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 commented 12 months ago

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 commented 12 months ago

What is TC?

namjaejeon commented 12 months ago

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.