| Message ID | 167319499150.7490.2294168831574653380.stgit@bazille.1015granger.net (mailing list archive) |
|---|---|
| Headers | show |
| Series | Server-side RPC reply header parsing overhaul | expand |
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>
> On Jan 10, 2023, at 9:53 AM, Jeff Layton <jlayton@kernel.org> wrote: > > 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? These are in the topic-rpcsec-gss-krb5-enhancements branch in my repo at kernel.org, although I'm about to push them to nfsd's for-next. > You can add: > > Reviewed-by: Jeff Layton <jlayton@kernel.org> I very much appreciate your time! -- Chuck Lever
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