[PATCH 07/28] gss_krb5: Use random value to initialize confounder

Kevin Coffman kwc at citi.umich.edu
Mon Mar 31 12:24:36 EDT 2008


On Mon, Mar 31, 2008 at 11:26 AM, Chuck Lever <chuck.lever at oracle.com> wrote:
> On Mar 31, 2008, at 10:31 AM, Kevin Coffman wrote:
>  > Initialize the value used for the confounder to a random value
>  > rather than starting from zero.
>  > Allow for confounders of length 8 or 16 (which will be needed for
>  > AES).
>  >
>  > Signed-off-by: Kevin Coffman <kwc at citi.umich.edu>
>  > ---
>  >
>  >  net/sunrpc/auth_gss/gss_krb5_wrap.c |   17 ++++++++++++++---
>  >  1 files changed, 14 insertions(+), 3 deletions(-)
>  >
>  > diff --git a/net/sunrpc/auth_gss/gss_krb5_wrap.c b/net/sunrpc/
>  > auth_gss/gss_krb5_wrap.c
>  > index 3cd99a7..89338a4 100644
>  > --- a/net/sunrpc/auth_gss/gss_krb5_wrap.c
>  > +++ b/net/sunrpc/auth_gss/gss_krb5_wrap.c
>  > @@ -88,7 +88,7 @@ out:
>  >  }
>  >
>  >  static inline void
>  > -make_confounder(char *p, int blocksize)
>  > +make_confounder(char *p, int conflen)
>
>  I would use an unsigned type for conflen... "size_t" is probably
>  better than "int".

Thanks.  I think size_t is overkill for this.  Is u32 acceptable?
That is what is
used elsewhere, anyway.

>  It might also be helpful to strip off the "inline" directive and let
>  the compiler decide whether inlining is a useful optimization.

O.K.

>  >  {
>  >       static u64 i = 0;
>  >       u64 *q = (u64 *)p;
>  > @@ -102,8 +102,19 @@ make_confounder(char *p, int blocksize)
>  >        * uniqueness would mean worrying about atomicity and rollover,
>  > and I
>  >        * don't care enough. */
>  >
>  > -     BUG_ON(blocksize != 8);
>  > -     *q = i++;
>  > +     /* initialize to random value */
>  > +     if (i == 0) {
>  > +             i = random32();
>  > +             i = (i << 32) | random32();
>  > +     }
>
>  Nit: The RPC layer usually uses net_random() instead of calling
>  random32() directly.

O.K.

>  > +
>  > +     if (conflen == 8)
>  > +             *q = i++;
>  > +     else if (conflen == 16) {
>  > +             *q++ = i++;
>  > +             *q++ = i++;
>  > +     } else
>  > +             BUG();
>
>  Nit: a "switch" would be more clear and slightly easier to maintain.

Fine by me.

>  >  }
>  >
>  >  /* Assumptions: the head and tail of inbuf are ours to play with.
>
>  --
>  Chuck Lever
>  chuck[dot]lever[at]oracle[dot]com
>
>


More information about the NFSv4 mailing list