Re: [PATCH 3/5] cifs: Make decode_unicode_ssetup use the helpers

On Fri, 17 Apr 2009 20:44:20 +0530
Suresh Jayaraman <[email protected]> wrote:

> In addition, remove the pointless "len >= words_left" check as
> rc is 0 always.
> 
> Signed-off-by: Suresh Jayaraman <[email protected]>
> ---
>  fs/cifs/sess.c |   51 ++++++++++++++++-----------------------------------
>  1 files changed, 16 insertions(+), 35 deletions(-)
> 
> diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c
> index c652c73..a54b254 100644
> --- a/fs/cifs/sess.c
> +++ b/fs/cifs/sess.c
> @@ -303,54 +303,35 @@ static int decode_unicode_ssetup(char **pbcc_area, int bleft,
>  	words_left = bleft / 2;
>  
>  	/* save off server operating system */
> -	len = UniStrnlen((wchar_t *) data, words_left);
> -
> -	if (len >= words_left)
> -		return rc;
> -
>  	kfree(ses->serverOS);
> -	/* UTF-8 string will not grow more than four times as big as UCS-16 */
> -	ses->serverOS = kzalloc((4 * len) + 2 /* trailing null */, GFP_KERNEL);
> -	if (ses->serverOS != NULL) {
> -		cifs_strfromUCS_le(ses->serverOS, (__le16 *)data, len, nls_cp);
> -		cFYI(1, ("serverOS=%s", ses->serverOS));
> -	}
> +	rc = cifs_ucs_to_nls(&(ses->serverOS), data, words_left, &len, nls_cp);
> +	if (rc)
> +		return rc;
> +	cFYI(1, ("serverOS=%s", ses->serverOS));
>  	data += 2 * (len + 1);
>  	words_left -= len + 1;
>  
>  	/* save off server network operating system */
> -	len = UniStrnlen((wchar_t *) data, words_left);
> -
> -	if (len >= words_left)
> -		return rc;
> -
>  	kfree(ses->serverNOS);
> -	ses->serverNOS = kzalloc((4 * len) + 2 /* trailing null */, GFP_KERNEL);
> -	if (ses->serverNOS != NULL) {
> -		cifs_strfromUCS_le(ses->serverNOS, (__le16 *)data, len,
> -				   nls_cp);
> -		cFYI(1, ("serverNOS=%s", ses->serverNOS));
> -		if (strncmp(ses->serverNOS, "NT LAN Manager 4", 16) == 0) {
> -			cFYI(1, ("NT4 server"));
> -			ses->flags |= CIFS_SES_NT4;
> -		}
> +	rc = cifs_ucs_to_nls(&(ses->serverNOS), data, words_left, &len, nls_cp);
> +	if (rc)
> +		return rc;
> +	cFYI(1, ("serverNOS=%s", ses->serverNOS));
> +	if (strncmp(ses->serverNOS, "NT LAN Manager 4", 16) == 0) {
> +		cFYI(1, ("NT4 server"));
> +		ses->flags |= CIFS_SES_NT4;
>  	}
>  	data += 2 * (len + 1);
>  	words_left -= len + 1;
>  
>  	/* save off server domain */
> -	len = UniStrnlen((wchar_t *) data, words_left);
> -
> -	if (len > words_left)

Win2k8 seems to skip sending a serverDomain string when I use sec=krb5.

Removing these checks is OK, but you need to ensure this code properly
handles the case where "words_left" goes to 0. It's possible I guess
that we may get 0, 1, or 2 strings here and I don't think we want to
throw an error in those cases.

> +	kfree(ses->serverDomain);
> +	rc = cifs_ucs_to_nls(&(ses->serverDomain), data, words_left, &len,
> +			     nls_cp);
> +	if (rc)
>  		return rc;
> +	cFYI(1, ("serverDomain=%s", ses->serverDomain));
>  
> -	kfree(ses->serverDomain);
> -	ses->serverDomain = kzalloc((4 * len) + 2, GFP_KERNEL);
> -	if (ses->serverDomain != NULL) {
> -		cifs_strfromUCS_le(ses->serverDomain, (__le16 *)data, len,
> -				   nls_cp);
> -		cFYI(1, ("serverDomain=%s", ses->serverDomain));
> -	}
>  	data += 2 * (len + 1);
>  	words_left -= len + 1;
>  
> _______________________________________________
> linux-cifs-client mailing list
> [email protected]
> https://lists.samba.org/mailman/listinfo/linux-cifs-client
> 


-- 
Jeff Layton <[email protected]>