Re: [GIT] [CIFS] Allow user names longer than 32 bytes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Arggghh

Good catch - thx.

On Mon, Feb 28, 2011 at 1:04 PM, Jeff Layton <jlayton@xxxxxxxxxx> wrote:
> On Mon, 28 Feb 2011 13:59:35 -0500
> Jeff Layton <jlayton@xxxxxxxxxx> wrote:
>
>> On Fri, 25 Feb 2011 12:24:17 -0600
>> Steve French <smfrench@xxxxxxxxx> wrote:
>>
>> > commit 355e57ca063338eb00ea067a7570bb5f136cc513
>> > Author: Steve French <sfrench@xxxxxxxxxx>
>> > Date:   Fri Feb 25 01:11:56 2011 -0600
>> >
>> >     [CIFS] Allow user names longer than 32 bytes
>> >
>> >     We artificially limited the user name to 32 bytes, but modern servers handle
>> >     larger.  Set the maximum length to a reasonable 256, and make the user name
>> >     string dynamically allocated rather than a fixed size in session structure.
>> >     Also clean up old checkpatch warning.
>> >
>> >     Signed-off-by: Steve French <sfrench@xxxxxxxxxx>
>>
>> [...]
>>
>> > diff --git a/fs/cifs/cifsencrypt.c b/fs/cifs/cifsencrypt.c
>> > index a51585f..e307a28 100644
>> > --- a/fs/cifs/cifsencrypt.c
>> > +++ b/fs/cifs/cifsencrypt.c
>> > @@ -469,15 +469,15 @@ static int calc_ntlmv2_hash(struct cifsSesInfo
>> > *ses, char *ntlmv2_hash,
>> >             return rc;
>> >     }
>> >
>> > -   /* convert ses->userName to unicode and uppercase */
>> > -   len = strlen(ses->userName);
>> > +   /* convert ses->user_name to unicode and uppercase */
>> > +   len = strlen(ses->user_name);
>> >     user = kmalloc(2 + (len * 2), GFP_KERNEL);
>> >     if (user == NULL) {
>> >             cERROR(1, "calc_ntlmv2_hash: user mem alloc failure\n");
>> >             rc = -ENOMEM;
>> >             goto calc_exit_2;
>> >     }
>> > -   len = cifs_strtoUCS((__le16 *)user, ses->userName, len, nls_cp);
>> > +   len = cifs_strtoUCS((__le16 *)user, ses->user_name, len, nls_cp);
>> >     UniStrupr(user);
>> >
>>
>> Now that user_name can be a NULL pointer, what prevents the code above
>> from oopsing if no one passes in a user= parm?
>>
>
> Oh, and another problem too...
>
> cifs_construct_tcon does a stack allocation for a temporary username field:
>
>        char username[MAX_USERNAME_SIZE + 1];
>
> That was probably ok when this was 33 bytes, but now that it's 257 it's
> a little more scary. That should probably be switched to a kzalloc'ed
> buffer too.
>
> --
> Jeff Layton <jlayton@xxxxxxxxxx>
>



-- 
Thanks,

Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux