Re: [PATCH v1 00/27] Server-side RPC reply header parsing overhaul

[Jeff Layton <jlayton@kernel.org>]

On Sun, 2023-01-08 at 11:28 -0500, Chuck Lever wrote:
> The purpose of this series is to replace the svc_put* macros in the
> Linux kernel server's RPC reply header construction code with
> xdr_stream helpers. I've measured no change in CPU utilization after
> the overhaul.
> 
> Memory safety: Buffer bounds checking after encoding each XDR item
> is more memory-safe than the current mechanism. Subsequent memory
> safety improvements to the common xdr_stream helpers will benefit
> all who use them.
> 
> Audit friendliness: The new code has additional comments and other
> clean-up to help align it with the relevant RPC protocol
> specifications. The use of common helpers also makes the encoders
> easier to audit and maintain.
> 
> I've split the full series in half to make it easier to review. The
> patches posted here are the second half, handling RPC reply header
> encoding.
> 
> Note that another benefit of this work is that we are taking one or
> two more strides closer to greater commonality between the client
> and server implementations of RPCSEC GSS.
> 
> ---
> 
> Chuck Lever (27):
>       SUNRPC: Clean up svcauth_gss_release()
>       SUNRPC: Rename automatic variables in svcauth_gss_wrap_resp_integ()
>       SUNRPC: Record gss_get_mic() errors in svcauth_gss_wrap_integ()
>       SUNRPC: Replace checksum construction in svcauth_gss_wrap_integ()
>       SUNRPC: Convert svcauth_gss_wrap_integ() to use xdr_stream()
>       SUNRPC: Rename automatic variables in svcauth_gss_wrap_resp_priv()
>       SUNRPC: Record gss_wrap() errors in svcauth_gss_wrap_priv()
>       SUNRPC: Add @head and @tail variables in svcauth_gss_wrap_priv()
>       SUNRPC: Convert svcauth_gss_wrap_priv() to use xdr_stream()
>       SUNRPC: Check rq_auth_stat when preparing to wrap a response
>       SUNRPC: Remove the rpc_stat variable in svc_process_common()
>       SUNRPC: Add XDR encoding helper for opaque_auth
>       SUNRPC: Push svcxdr_init_encode() into svc_process_common()
>       SUNRPC: Move svcxdr_init_encode() into ->accept methods
>       SUNRPC: Use xdr_stream to encode Reply verifier in svcauth_null_accept()
>       SUNRPC: Use xdr_stream to encode Reply verifier in svcauth_unix_accept()
>       SUNRPC: Use xdr_stream to encode Reply verifier in svcauth_tls_accept()
>       SUNRPC: Convert unwrap data paths to use xdr_stream for replies
>       SUNRPC: Use xdr_stream to encode replies in server-side GSS upcall helpers
>       SUNRPC: Use xdr_stream for encoding GSS reply verifiers
>       SUNRPC: Hoist init_encode out of svc_authenticate()
>       SUNRPC: Convert RPC Reply header encoding to use xdr_stream
>       SUNRPC: Final clean-up of svc_process_common()
>       SUNRPC: Remove no-longer-used helper functions
>       SUNRPC: Refactor RPC server dispatch method
>       SUNRPC: Set rq_accept_statp inside ->accept methods
>       SUNRPC: Go back to using gsd->body_start
> 
> 
>  fs/lockd/svc.c                    |   5 +-
>  fs/nfs/callback_xdr.c             |   6 +-
>  fs/nfsd/nfscache.c                |   4 +-
>  fs/nfsd/nfsd.h                    |   2 +-
>  fs/nfsd/nfssvc.c                  |  10 +-
>  include/linux/sunrpc/svc.h        | 116 +++----
>  include/linux/sunrpc/xdr.h        |  23 ++
>  include/trace/events/rpcgss.h     |  22 ++
>  net/sunrpc/auth_gss/svcauth_gss.c | 505 +++++++++++++++---------------
>  net/sunrpc/svc.c                  |  91 +++---
>  net/sunrpc/svcauth_unix.c         |  40 ++-
>  net/sunrpc/xdr.c                  |  29 ++
>  12 files changed, 451 insertions(+), 402 deletions(-)
> 
> --
> Chuck Lever
> 

I went through the whole set and this all looks like good stuff to me.
The result is a lot more readable, and there is a lot less manual
fiddling with buffer lengths and such.

Do you have a public branch with the current state of this set?

You can add:

Reviewed-by: Jeff Layton <jlayton@kernel.org>