Re: [PATCH 3/5] cifs: Fix incorrect destination buffer size in cifs_strncpy_to_host (Try #2)

On Thu, 07 May 2009 10:33:13 +0530
Suresh Jayaraman <[email protected]> wrote:

> I think it is also important to fix an existing problem - using
> src buffer length to NULL terminate dst buffer, while at it.
> Jeff's patchset uses newly introduced helpers to fix this.
> 
> So, here is the revised patch.
> 
> Selected minimal hunks of commit 968460ebd8006d55661dec0fb86712b40d71c413.
> Also fix an existing problem pointed out by Guenter Kukuk that length of src
> is used for NULL termination of dst.
> 
> Signed-off-by: Suresh Jayaraman <[email protected]>
> ---
>  fs/cifs/cifssmb.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Index: linux-2.6.29.2/fs/cifs/cifssmb.c
> ===================================================================
> --- linux-2.6.29.2.orig/fs/cifs/cifssmb.c
> +++ linux-2.6.29.2/fs/cifs/cifssmb.c
> @@ -91,23 +91,22 @@ static int
>  cifs_strncpy_to_host(char **dst, const char *src, const int maxlen,
>  		 const bool is_unicode, const struct nls_table *nls_codepage)
>  {
> -	int plen;
> +	int src_len, dst_len;
>  
>  	if (is_unicode) {
> -		plen = UniStrnlen((wchar_t *)src, maxlen);
> -		*dst = kmalloc(plen + 2, GFP_KERNEL);
> +		src_len = UniStrnlen((wchar_t *)src, maxlen);
> +		*dst = kmalloc((4 * src_len) + 2, GFP_KERNEL);
>  		if (!*dst)
>  			goto cifs_strncpy_to_host_ErrExit;
> -		cifs_strfromUCS_le(*dst, (__le16 *)src, plen, nls_codepage);
> +		dst_len = cifs_strfromUCS_le(*dst, (__le16 *)src, src_len, nls_codepage);
> +		(*dst)[dst_len + 1] = 0;
>  	} else {
> -		plen = strnlen(src, maxlen);
> -		*dst = kmalloc(plen + 2, GFP_KERNEL);
> +		src_len = strnlen(src, maxlen);
> +		*dst = kmalloc(src_len + 1, GFP_KERNEL);
>  		if (!*dst)
>  			goto cifs_strncpy_to_host_ErrExit;
> -		strncpy(*dst, src, plen);
> +		strlcpy(*dst, src, src_len + 1);
>  	}
> -	(*dst)[plen] = 0;
> -	(*dst)[plen+1] = 0; /* harmless for ASCII case, needed for Unicode */
>  	return 0;
>  
>  cifs_strncpy_to_host_ErrExit:
> 
> Suresh Jayaraman wrote:
> > adding missing S-O-B's
> > 
> > 
> > Suresh Jayaraman wrote:
> >> From: Suresh Jayaraman <[email protected]>
> >> Subject: Fix incorrect destination buffer size in cifs_strncpy_to_host
> >>
> >> Selected minimal hunks of commit 968460ebd8006d55661dec0fb86712b40d71c413
> > 
> > Signed-off-by: Suresh Jayaraman <[email protected]>
> > Acked-by: Jeff Layton <[email protected]>
> > Signed-off-by: Steve French <[email protected]>
> > 
> >> ---
> >>  fs/cifs/cifssmb.c |    2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> Index: linux-2.6.29.2/fs/cifs/cifssmb.c
> >> ===================================================================
> >> --- linux-2.6.29.2.orig/fs/cifs/cifssmb.c
> >> +++ linux-2.6.29.2/fs/cifs/cifssmb.c
> >> @@ -95,7 +95,7 @@ cifs_strncpy_to_host(char **dst, const c
> >>  
> >>  	if (is_unicode) {
> >>  		plen = UniStrnlen((wchar_t *)src, maxlen);
> >> -		*dst = kmalloc(plen + 2, GFP_KERNEL);
> >> +		*dst = kmalloc((4 * plen) + 2, GFP_KERNEL);
> >>  		if (!*dst)
> >>  			goto cifs_strncpy_to_host_ErrExit;
> >>  		cifs_strfromUCS_le(*dst, (__le16 *)src, plen, nls_codepage);
> >> _______________________________________________
> >> linux-cifs-client mailing list
> >> [email protected]
> >> https://lists.samba.org/mailman/listinfo/linux-cifs-client
> > 
> > 
> 
> 
> -- 
> Suresh Jayaraman

Acked-by: Jeff Layton <[email protected]>