> On May 2, 2023, at 7:01 AM, Jiri Slaby <jirislaby@xxxxxxxxxx> wrote:
>
> On 08. 01. 23, 17:31, Chuck Lever wrote:
>> From: Chuck Lever <chuck.lever@xxxxxxxxxx>
>> To navigate around the space that svcauth_gss_accept() reserves
>> for the RPC payload body length and sequence number fields,
>> svcauth_gss_release() does a little dance with the reply's
>> accept_stat, moving the accept_stat value in the response buffer
>> down by two words.
>> Instead, let's have the ->accept() methods each set the proper
>> final location of the accept_stat to avoid having to move
>> things.
>
> Hi,
>
> I bisected to this (4bcf0343e8)
Assuming you did the bisect on the NFS server's kernel?
> as it breaks nfs3-only servers in 6.3. I.e. /etc/nfs.conf containing:
> [nfsd]
> vers4=no
Note: Changing the settings in /etc/nfs.conf had no effect
on my server, so I effected the change by stopping the
server and poking values into /proc/fs/nfsd/versions by
hand.
Steve?
> The client sees:
> mount("10.0.2.15:/tmp", "/mnt", "nfs", 0, "vers=4.2,addr=10.0.2.15,clientad"...) = -1 EIO (Input/output error)
> write(2, "mount.nfs: mount system call fai"..., 45
> mount.nfs: mount system call failed for /mnt
>
> And the kernel says:
> nfs4_discover_server_trunking unhandled error -5. Exiting with error EIO
>
> I reported in downstream as:
> https://bugzilla.suse.com/show_bug.cgi?id=1210995
>
> It cannot be reverted cleanly on the top of 6.3.
>
> Any ideas?
I can reproduce a similar problem. Network capture shows
that the server is responding with NFS4ERR_NOENT to the
EXCHANGE_ID operation, and the client kernel log says:
> nfs4_discover_server_trunking unhandled error -121. Exiting with error EIO
That's not the failure mode I expected given the commit
you bisected to, so it might not be the same problem you've
hit. I'll troubleshoot this and send a fix for testing.
> Thanks.
>
>> Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
>> ---
>> include/linux/sunrpc/svc.h | 19 +++++++++++++++++++
>> net/sunrpc/auth_gss/svcauth_gss.c | 21 ++++++++++-----------
>> net/sunrpc/svc.c | 2 --
>> net/sunrpc/svcauth_unix.c | 6 ++++++
>> 4 files changed, 35 insertions(+), 13 deletions(-)
>> diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h
>> index f40a90ca5de6..392d2d2620fa 100644
>> --- a/include/linux/sunrpc/svc.h
>> +++ b/include/linux/sunrpc/svc.h
>> @@ -544,4 +544,23 @@ static inline void svcxdr_set_auth_slack(struct svc_rqst *rqstp, int slack)
>> WARN_ON(xdr->p > xdr->end);
>> }
>> +/**
>> + * svcxdr_set_accept_stat - Reserve space for the accept_stat field
>> + * @rqstp: RPC transaction context
>> + *
>> + * Return values:
>> + * %true: Success
>> + * %false: No response buffer space was available
>> + */
>> +static inline bool svcxdr_set_accept_stat(struct svc_rqst *rqstp)
>> +{
>> + struct xdr_stream *xdr = &rqstp->rq_res_stream;
>> +
>> + rqstp->rq_accept_statp = xdr_reserve_space(xdr, XDR_UNIT);
>> + if (unlikely(!rqstp->rq_accept_statp))
>> + return false;
>> + *rqstp->rq_accept_statp = rpc_success;
>> + return true;
>> +}
>> +
>> #endif /* SUNRPC_SVC_H */
>> diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
>> index 560fb8a2803d..333873abb7d9 100644
>> --- a/net/sunrpc/auth_gss/svcauth_gss.c
>> +++ b/net/sunrpc/auth_gss/svcauth_gss.c
>> @@ -1220,7 +1220,7 @@ svcauth_gss_legacy_init(struct svc_rqst *rqstp,
>> if (!svcauth_gss_proc_init_verf(sn->rsc_cache, rqstp, &rsip->out_handle,
>> &rsip->major_status, GSS_SEQ_WIN))
>> goto out;
>> - if (xdr_stream_encode_u32(&rqstp->rq_res_stream, RPC_SUCCESS) < 0)
>> + if (!svcxdr_set_accept_stat(rqstp))
>> goto out;
>> if (!svcxdr_encode_gss_init_res(&rqstp->rq_res_stream, &rsip->out_handle,
>> &rsip->out_token, rsip->major_status,
>> @@ -1348,7 +1348,7 @@ static int svcauth_gss_proxy_init(struct svc_rqst *rqstp,
>> if (!svcauth_gss_proc_init_verf(sn->rsc_cache, rqstp, &cli_handle,
>> &ud.major_status, GSS_SEQ_WIN))
>> goto out;
>> - if (xdr_stream_encode_u32(&rqstp->rq_res_stream, RPC_SUCCESS) < 0)
>> + if (!svcxdr_set_accept_stat(rqstp))
>> goto out;
>> if (!svcxdr_encode_gss_init_res(&rqstp->rq_res_stream, &cli_handle,
>> &ud.out_token, ud.major_status,
>> @@ -1640,16 +1640,18 @@ svcauth_gss_accept(struct svc_rqst *rqstp)
>> case RPC_GSS_PROC_DESTROY:
>> if (!svcauth_gss_encode_verf(rqstp, rsci->mechctx, gc->gc_seq))
>> goto auth_err;
>> + if (!svcxdr_set_accept_stat(rqstp))
>> + goto auth_err;
>> /* Delete the entry from the cache_list and call cache_put */
>> sunrpc_cache_unhash(sn->rsc_cache, &rsci->h);
>> - if (xdr_stream_encode_u32(&rqstp->rq_res_stream, RPC_SUCCESS) < 0)
>> - goto auth_err;
>> goto complete;
>> case RPC_GSS_PROC_DATA:
>> rqstp->rq_auth_stat = rpcsec_gsserr_ctxproblem;
>> svcdata->verf_start = xdr_reserve_space(&rqstp->rq_res_stream, 0);
>> if (!svcauth_gss_encode_verf(rqstp, rsci->mechctx, gc->gc_seq))
>> goto auth_err;
>> + if (!svcxdr_set_accept_stat(rqstp))
>> + goto auth_err;
>> rqstp->rq_cred = rsci->cred;
>> get_group_info(rsci->cred.cr_group_info);
>> rqstp->rq_auth_stat = rpc_autherr_badcred;
>> @@ -1706,7 +1708,6 @@ svcauth_gss_accept(struct svc_rqst *rqstp)
>> static __be32 *
>> svcauth_gss_prepare_to_wrap(struct svc_rqst *rqstp, struct gss_svc_data *gsd)
>> {
>> - struct xdr_buf *resbuf = &rqstp->rq_res;
>> __be32 *p;
>> u32 verf_len;
>> @@ -1721,13 +1722,11 @@ svcauth_gss_prepare_to_wrap(struct svc_rqst *rqstp, struct gss_svc_data *gsd)
>> p += 1;
>> verf_len = ntohl(*p++);
>> p += XDR_QUADLEN(verf_len);
>> - /* move accept_stat to right place: */
>> - memcpy(p, p + 2, 4);
>> - /* Also don't wrap if the accept stat is nonzero: */
>> - if (*p != rpc_success) {
>> - resbuf->head[0].iov_len -= 2 * 4;
>> +
>> + /* Also don't wrap if the accept_stat is nonzero: */
>> + if (*rqstp->rq_accept_statp != rpc_success)
>> return NULL;
>> - }
>> +
>> p++;
>> return p;
>> }
>> diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
>> index 3c194e6f8f5e..c2ed8b06fadb 100644
>> --- a/net/sunrpc/svc.c
>> +++ b/net/sunrpc/svc.c
>> @@ -1314,8 +1314,6 @@ svc_process_common(struct svc_rqst *rqstp)
>> trace_svc_process(rqstp, progp->pg_name);
>> aoffset = xdr_stream_pos(xdr);
>> - rqstp->rq_accept_statp = xdr_reserve_space(&rqstp->rq_res_stream, XDR_UNIT);
>> - *rqstp->rq_accept_statp = rpc_success;
>> /* un-reserve some of the out-queue now that we have a
>> * better idea of reply size
>> diff --git a/net/sunrpc/svcauth_unix.c b/net/sunrpc/svcauth_unix.c
>> index b101700d155c..62dfc8cdf8c5 100644
>> --- a/net/sunrpc/svcauth_unix.c
>> +++ b/net/sunrpc/svcauth_unix.c
>> @@ -775,6 +775,8 @@ svcauth_null_accept(struct svc_rqst *rqstp)
>> if (xdr_stream_encode_opaque_auth(&rqstp->rq_res_stream,
>> RPC_AUTH_NULL, NULL, 0) < 0)
>> return SVC_CLOSE;
>> + if (!svcxdr_set_accept_stat(rqstp))
>> + return SVC_CLOSE;
>> rqstp->rq_cred.cr_flavor = RPC_AUTH_NULL;
>> return SVC_OK;
>> @@ -866,6 +868,8 @@ svcauth_tls_accept(struct svc_rqst *rqstp)
>> RPC_AUTH_NULL, NULL, 0) < 0)
>> return SVC_CLOSE;
>> }
>> + if (!svcxdr_set_accept_stat(rqstp))
>> + return SVC_CLOSE;
>> rqstp->rq_cred.cr_flavor = RPC_AUTH_TLS;
>> return SVC_OK;
>> @@ -960,6 +964,8 @@ svcauth_unix_accept(struct svc_rqst *rqstp)
>> if (xdr_stream_encode_opaque_auth(&rqstp->rq_res_stream,
>> RPC_AUTH_NULL, NULL, 0) < 0)
>> return SVC_CLOSE;
>> + if (!svcxdr_set_accept_stat(rqstp))
>> + return SVC_CLOSE;
>> rqstp->rq_cred.cr_flavor = RPC_AUTH_UNIX;
>> return SVC_OK;
>
> --
> js
> suse labs
--
Chuck Lever
