On Thu, Jan 21, 2021 at 12:05 PM Trond Myklebust
<trondmy@xxxxxxxxxxxxxxx> wrote:
>
> On Thu, 2021-01-21 at 11:20 -0500, Dave Wysochanski wrote:
> > Remove duplicated helper functions to parse opaque XDR objects
> > and place inside the xdr.h file. Also update comment which
> > is wrong since lockd is not the only user of xdr_netobj.
> >
> > Signed-off-by: Dave Wysochanski <dwysocha@xxxxxxxxxx>
> > ---
> > include/linux/sunrpc/xdr.h | 33
> > +++++++++++++++++++++++++++++++--
> > net/sunrpc/auth_gss/auth_gss.c | 29 ---------------------------
> > --
> > net/sunrpc/auth_gss/gss_krb5_mech.c | 29 ---------------------------
> > --
> > 3 files changed, 31 insertions(+), 60 deletions(-)
> >
> > diff --git a/include/linux/sunrpc/xdr.h b/include/linux/sunrpc/xdr.h
> > index 19b6dea27367..8ef788ff80b9 100644
> > --- a/include/linux/sunrpc/xdr.h
> > +++ b/include/linux/sunrpc/xdr.h
> > @@ -25,8 +25,7 @@
> > #define XDR_QUADLEN(l) (((l) + 3) >> 2)
> >
> > /*
> > - * Generic opaque `network object.' At the kernel level, this type
> > - * is used only by lockd.
> > + * Generic opaque `network object.'
> > */
> > #define XDR_MAX_NETOBJ 1024
> > struct xdr_netobj {
> > @@ -34,6 +33,36 @@ struct xdr_netobj {
> > u8 * data;
> > };
> >
> > +static inline const void *
> > +simple_get_bytes(const void *p, const void *end, void *res, size_t
> > len)
> > +{
> > + const void *q = (const void *)((const char *)p + len);
> > + if (unlikely(q > end || q < p))
> > + return ERR_PTR(-EFAULT);
> > + memcpy(res, p, len);
> > + return q;
> > +}
> > +
> > +static inline const void *
> > +simple_get_netobj(const void *p, const void *end, struct xdr_netobj
> > *dest)
> > +{
> > + const void *q;
> > + unsigned int len;
> > +
> > + p = simple_get_bytes(p, end, &len, sizeof(len));
> > + if (IS_ERR(p))
> > + return p;
> > + q = (const void *)((const char *)p + len);
> > + if (unlikely(q > end || q < p))
> > + return ERR_PTR(-EFAULT);
> > + dest->data = kmemdup(p, len, GFP_NOFS);
> > + if (unlikely(dest->data == NULL))
> > + return ERR_PTR(-ENOMEM);
> > + dest->len = len;
> > + return q;
> > +}
> > +
>
> Hmm... I'm not too keen on having these in sunrpc/xdr.h. For one thing,
> they do not describe objects that are XDR encoded. Secondly, their
> naming isn't really consistent with any of the existing conventions for
> xdr.
>
> Can we please rather just create a new header file that is private in
> net/sunrpc/auth_gss/ ?
>
Sure. Do you have any preference for the name?
internal.h
auth_gss.h
something else...
> > +
> > /*
> > * Basic structure for transmission/reception of a client XDR
> > message.
> > * Features a header (for a linear buffer containing RPC headers
> > diff --git a/net/sunrpc/auth_gss/auth_gss.c
> > b/net/sunrpc/auth_gss/auth_gss.c
> > index 4ecc2a959567..228456b22b23 100644
> > --- a/net/sunrpc/auth_gss/auth_gss.c
> > +++ b/net/sunrpc/auth_gss/auth_gss.c
> > @@ -125,35 +125,6 @@ struct gss_auth {
> > clear_bit(RPCAUTH_CRED_NEW, &cred->cr_flags);
> > }
> >
> > -static const void *
> > -simple_get_bytes(const void *p, const void *end, void *res, size_t
> > len)
> > -{
> > - const void *q = (const void *)((const char *)p + len);
> > - if (unlikely(q > end || q < p))
> > - return ERR_PTR(-EFAULT);
> > - memcpy(res, p, len);
> > - return q;
> > -}
> > -
> > -static inline const void *
> > -simple_get_netobj(const void *p, const void *end, struct xdr_netobj
> > *dest)
> > -{
> > - const void *q;
> > - unsigned int len;
> > -
> > - p = simple_get_bytes(p, end, &len, sizeof(len));
> > - if (IS_ERR(p))
> > - return p;
> > - q = (const void *)((const char *)p + len);
> > - if (unlikely(q > end || q < p))
> > - return ERR_PTR(-EFAULT);
> > - dest->data = kmemdup(p, len, GFP_NOFS);
> > - if (unlikely(dest->data == NULL))
> > - return ERR_PTR(-ENOMEM);
> > - dest->len = len;
> > - return q;
> > -}
> > -
> > static struct gss_cl_ctx *
> > gss_cred_get_ctx(struct rpc_cred *cred)
> > {
> > diff --git a/net/sunrpc/auth_gss/gss_krb5_mech.c
> > b/net/sunrpc/auth_gss/gss_krb5_mech.c
> > index ae9acf3a7389..99ea36d5eefe 100644
> > --- a/net/sunrpc/auth_gss/gss_krb5_mech.c
> > +++ b/net/sunrpc/auth_gss/gss_krb5_mech.c
> > @@ -143,35 +143,6 @@
> > return NULL;
> > }
> >
> > -static const void *
> > -simple_get_bytes(const void *p, const void *end, void *res, int len)
> > -{
> > - const void *q = (const void *)((const char *)p + len);
> > - if (unlikely(q > end || q < p))
> > - return ERR_PTR(-EFAULT);
> > - memcpy(res, p, len);
> > - return q;
> > -}
> > -
> > -static const void *
> > -simple_get_netobj(const void *p, const void *end, struct xdr_netobj
> > *res)
> > -{
> > - const void *q;
> > - unsigned int len;
> > -
> > - p = simple_get_bytes(p, end, &len, sizeof(len));
> > - if (IS_ERR(p))
> > - return p;
> > - q = (const void *)((const char *)p + len);
> > - if (unlikely(q > end || q < p))
> > - return ERR_PTR(-EFAULT);
> > - res->data = kmemdup(p, len, GFP_NOFS);
> > - if (unlikely(res->data == NULL))
> > - return ERR_PTR(-ENOMEM);
> > - res->len = len;
> > - return q;
> > -}
> > -
> > static inline const void *
> > get_key(const void *p, const void *end,
> > struct krb5_ctx *ctx, struct crypto_sync_skcipher **res)
>
> --
> Trond Myklebust
> Linux NFS client maintainer, Hammerspace
> trond.myklebust@xxxxxxxxxxxxxxx
>
>
