Re: [PATCH v3 2/2] ksmbd: fix named-pipe reply loss with small output buffers

[Namjae Jeon]

On Tue, Oct 27, 2026 at 12:20 AM Wang Zhaolong
<wangzhaolong633@xxxxxxxxx> wrote:
>
> Hi Namjae,
Hi Wang,
>
> >
> > if (rpc->pending)
> >     rsp->hdr.Status = STATUS_BUFFER_OVERFLOW;
> > Should we add this check ?
> >
> >> +       goto done;
> >>
> >> +empty:
> >> +       err = ksmbd_iov_pin_rsp(work, rsp,
> >> +                               offsetof(struct smb2_read_rsp, Buffer));
> >> +       if (err)
> >> +               goto out_put;
> >> +done:
> >>         rsp->StructureSize = cpu_to_le16(17);
> >>         rsp->DataOffset = 80;
> >>         rsp->Reserved = 0;
> >> @@ -9349,7 +9375,6 @@ static noinline int smb2_read_pipe(struct ksmbd_work *work)
> >>         rsp->hdr.Status = smb2_pipe_error_status(err,
> >>                                                 STATUS_UNEXPECTED_IO_ERROR);
> >>         smb2_set_err_rsp(work);
> >> -       kvfree(rpc_resp);
> >>         return err;
> >>  }
> >>
> >> @@ -9947,7 +9972,7 @@ static noinline int smb2_write_pipe(struct ksmbd_work *work)
> >>         if (IS_ERR(rpc)) {
> >>                 err = PTR_ERR(rpc);
> >>                 rsp->hdr.Status = smb2_pipe_error_status(err,
> >> -                                                       STATUS_INVALID_HANDLE);
> >> +                                                        STATUS_INVALID_HANDLE);
> >>                 smb2_set_err_rsp(work);
> >>                 return err;
> >>         }
> >> @@ -11440,20 +11465,25 @@ static int fsctl_pipe_transceive(struct ksmbd_work *work, u64 id,
> >>         struct ksmbd_rpc_command *rpc_resp;
> >>         struct ksmbd_session_rpc *rpc;
> >>         char *data_buf = (char *)req + le32_to_cpu(req->InputOffset);
> >> -       unsigned int nbytes;
> >> -       int err;
> >> +       unsigned int nbytes = 0;
> >> +       int ret;
> >>
> >>         rpc = ksmbd_session_rpc_get(work, id);
> >>         if (IS_ERR(rpc)) {
> >> -               err = PTR_ERR(rpc);
> >> -               rsp->hdr.Status = smb2_pipe_error_status(err,
> >> -                                                       STATUS_INVALID_PARAMETER);
> >> +               ret = PTR_ERR(rpc);
> >> +               rsp->hdr.Status = smb2_pipe_error_status(ret,
> >> +                                                STATUS_INVALID_PARAMETER);
> >>                 smb2_set_err_rsp(work);
> >>                 return 0;
> >>         }
> >>
> >> -       rpc_resp = ksmbd_rpc_ioctl(work->sess, rpc, data_buf,
> >> -                                  le32_to_cpu(req->InputCount));
> >> +       /* Write new input without reading past an already buffered reply. */
> >> +       if (rpc->pending)
> >> +               rpc_resp = ksmbd_rpc_write(work->sess, rpc, data_buf,
> >> +                                          le32_to_cpu(req->InputCount));
> >> +       else
> >> +               rpc_resp = ksmbd_rpc_ioctl(work->sess, rpc, data_buf,
> >> +                                          le32_to_cpu(req->InputCount));
> > This WRITE leaves KSMBD_DCERPC_RETURN_READY set in mountd. After the
> > cached reply is drained, the next TRANSCEIVE can return the previous
> > reply without processing its own input.
> >
> > Thanks.
>
>
> Thanks for catching this.
>
> I reproduced the sequence you described and confirmed the problem.
>
> For the READ path, your suggested STATUS_BUFFER_OVERFLOW check looks
> appropriate when a positive-length read leaves unread bytes from
> the same reply.
>
> For TRANSCEIVE, I think the correct fix needs some discussion.
> One possibility is to return STATUS_PIPE_BUSY when cached reply
> data remains, without issuing another IPC request or consuming the
> cached data. The client would need to drain that reply through READ
> before submitting another transaction. MS-FSCC §2.3.48[1] lists
> PIPE_BUSY for a pipe containing unread data.
>
> However, I would like to confirm that this is the intended behavior
> for this SMB2 path before preparing v4. Simply omitting the WRITE
> and returning cached data would still silently discard the input.
>
> Would rejecting the transaction with STATUS_PIPE_BUSY be appropriate
> here, or would you prefer a different approach?
I think the former is correct.

Thanks.
>
> [1] https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-fscc/f48cecf1-98d8-48dc-926f-2f3a48f07cf6
>
> Thanks,
> Wang Zhaolong
>