[linux-cifs-client] Re: [PATCH] [CIFS] properly account for new user= field in SPNEGO upcall string allocation

[Steve French]

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.

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(-)
>



-- 
Thanks,

Steve