[linux-cifs-client] Re: [PATCH] [CIFS] properly account for new
user= field in SPNEGO upcall string allocation
Jeff Layton
jlayton at redhat.com
Fri Aug 1 18:23:47 GMT 2008
On Fri, 1 Aug 2008 13:08:14 -0500
"Steve French" <smfrench at gmail.com> wrote:
> Jeff,
> Yes it should go in stable. Note that I modified your patch with one
> minor change. I don't know whether it needs to go in 2.6.25 stable as
> well as 2.6.26-stable though (can you check). Note that I added +1 to
> the length to account for a possible trailing null, and added some
> comments explaining the length calculation.
>
Thanks Steve.
Looks like it's just a problem in 2.6.26. 2.6.25 looks like it predates
the addition of "user=" to the upcall string.
The new patch looks good to me. I thought about adding some more
detailed comments after I sent the patch yesterday, but got sidetracked.
In practice, I think this problem is usually papered over by the fact
IPv6 and MAX_IPV6_ADDR_LEN is pretty long, but it certainly wouldn't
hurt to fix it the right way.
> commit 66b8bd3c405389213de1d6ba6c2565990f62004f
> Author: Jeff Layton <jlayton at redhat.com>
> Date: Fri Aug 1 17:54:32 2008 +0000
>
> [CIFS] properly account for new user= field in SPNEGO upcall
> string allocation
>
> ...it doesn't look like it's being accounted for at the moment. Also
> try to reorganize the calculation to make it a little more evident
> what each piece means.
>
> This should probably go to the stable series as well...
>
> Signed-off-by: Jeff Layton <jlayton at redhat.com>
> Signed-off-by: Steve French <sfrench at us.ibm.com>
>
> diff --git a/fs/cifs/cifs_spnego.c b/fs/cifs/cifs_spnego.c
> index 7013aaf..2434ab0 100644
> --- a/fs/cifs/cifs_spnego.c
> +++ b/fs/cifs/cifs_spnego.c
> @@ -66,8 +66,8 @@ struct key_type cifs_spnego_key_type = {
> .describe = user_describe,
> };
>
> -#define MAX_VER_STR_LEN 9 /* length of longest version string e.g.
> - strlen(";ver=0xFF") */
> +#define MAX_VER_STR_LEN 8 /* length of longest version string e.g.
> + strlen("ver=0xFF") */
> #define MAX_MECH_STR_LEN 13 /* length of longest security mechanism name, eg
> in future could have strlen(";sec=ntlmsspi") */
> #define MAX_IPV6_ADDR_LEN 42 /* eg
> FEDC:BA98:7654:3210:FEDC:BA98:7654:3210/60 */
> @@ -81,11 +81,15 @@ cifs_get_spnego_key(struct cifsSesInfo *sesInfo)
> struct key *spnego_key;
> const char *hostname = server->hostname;
>
> - /* BB: come up with better scheme for determining length */
> - /* length of fields (with semicolons): ver=0xyz ipv4= ipaddress host=
> - hostname sec=mechanism uid=0x uid */
> - desc_len = MAX_VER_STR_LEN + 5 + MAX_IPV6_ADDR_LEN + 1 + 6 +
> - strlen(hostname) + MAX_MECH_STR_LEN + 8 + (sizeof(uid_t) * 2);
> + /* length of fields (with semicolons): ver=0xyz ip4=ipaddress
> + host=hostname sec=mechanism uid=0xFF user=username */
> + desc_len = MAX_VER_STR_LEN +
> + 6 /* len of "host=" */ + strlen(hostname) +
> + 5 /* len of ";ipv4=" */ + MAX_IPV6_ADDR_LEN +
> + MAX_MECH_STR_LEN +
> + 7 /* len of ";uid=0x" */ + (sizeof(uid_t) * 2) +
> + 6 /* len of ";user=" */ + strlen(sesInfo->userName) + 1;
> +
> spnego_key = ERR_PTR(-ENOMEM);
> description = kzalloc(desc_len, GFP_KERNEL);
> if (description == NULL)
>
>
> On Thu, Jul 31, 2008 at 3:26 PM, Jeff Layton <jlayton at redhat.com> wrote:
> > ...it doesn't look like it's being accounted for at the moment. Also
> > try to reorganize the calculation to make it a little more evident
> > what each piece means.
> >
> > Steve, this should probably go to the stable series as well...
> >
> > Signed-off-by: Jeff Layton <jlayton at redhat.com>
> > ---
> >
> > fs/cifs/cifs_spnego.c | 17 +++++++++++------
> > 1 files changed, 11 insertions(+), 6 deletions(-)
> >
>
>
>
--
Jeff Layton <jlayton at redhat.com>
More information about the linux-cifs-client
mailing list