Re: git: b484bcd504a2 - main - nfscl: Fix handling of a copyout() error reply

Just FYI, I missed the MCF after on this commit.
I plan to MFC it in a week.

rick

On Fri, Dec 22, 2023 at 12:13 PM Rick Macklem <rmacklem@freebsd.org> wrote:
>
> CAUTION: This email originated from outside of the University of Guelph. Do not click links or open attachments unless you recognize the sender and know the content is safe. If in doubt, forward suspicious emails to IThelp@uoguelph.ca.
>
>
> The branch main has been updated by rmacklem:
>
> URL: https://cgit.FreeBSD.org/src/commit/?id=b484bcd504a29037752d5214a418412724761d88
>
> commit b484bcd504a29037752d5214a418412724761d88
> Author:     Rick Macklem <rmacklem@FreeBSD.org>
> AuthorDate: 2023-12-22 20:11:22 +0000
> Commit:     Rick Macklem <rmacklem@FreeBSD.org>
> CommitDate: 2023-12-22 20:11:22 +0000
>
>     nfscl: Fix handling of a copyout() error reply
>
>     If vfs.nfs.nfs_directio_enable is set non-zero (the default is
>     zero) and a file on an NFS mount is read after being opened
>     with O_DIRECT | O_ RDONLY, a call to nfsm_mbufuio() calls
>     copyout() without checking for an error return.
>     If copyout() returns EFAULT, this would not work correctly.
>
>     Only the call path
>      VOP_READ()->ncl_readrpc()->nfsrpc_read()->nfsrpc_readrpc()
>     will do this and the error return for EFAULT will
>     be returned back to VOP_READ().
>
>     This patch adds the error check to nfsm_mbufuio().
>
>     Reviewed by:    markj
>     Differential Revision:  https://reviews.freebsd.org/D43160
> ---
>  sys/fs/nfs/nfs_commonsubs.c | 14 +++++---------
>  1 file changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/sys/fs/nfs/nfs_commonsubs.c b/sys/fs/nfs/nfs_commonsubs.c
> index 832713e6c1de..e79f73739487 100644
> --- a/sys/fs/nfs/nfs_commonsubs.c
> +++ b/sys/fs/nfs/nfs_commonsubs.c
> @@ -679,17 +679,13 @@ nfsm_mbufuio(struct nfsrv_descript *nd, struct uio *uiop, int siz)
>                                     ("len %d, corrupted mbuf?", len));
>                         }
>                         xfer = (left > len) ? len : left;
> -#ifdef notdef
> -                       /* Not Yet.. */
> -                       if (uiop->uio_iov->iov_op != NULL)
> -                               (*(uiop->uio_iov->iov_op))
> -                               (mbufcp, uiocp, xfer);
> -                       else
> -#endif
>                         if (uiop->uio_segflg == UIO_SYSSPACE)
>                                 NFSBCOPY(mbufcp, uiocp, xfer);
> -                       else
> -                               copyout(mbufcp, uiocp, xfer);
> +                       else {
> +                               error = copyout(mbufcp, uiocp, xfer);
> +                               if (error != 0)
> +                                       goto out;
> +                       }
>                         left -= xfer;
>                         len -= xfer;
>                         mbufcp += xfer;