Re: [PATCH 1/2] cifs: Rename cifs_strncpy_to_host and fix buffer size

On Mon, 20 Apr 2009 18:54:21 +0530
Suresh Jayaraman <[email protected]> wrote:

> There is a possibility for the path_name and node_name buffers to
> overflow if they contain charcters that are >2 bytes in the local
> charset. Resize the buffer allocation so to avoid this possibility.
> 
> Also, as pointed out by Jeff Layton, it would be appropriate to
> rename the function to cifs_strlcpy_to_host to reflect the fact
> that the copied string is always NULL terminated.
> 
> 
> Signed-off-by: Suresh Jayaraman <[email protected]>
> ---
>  fs/cifs/cifssmb.c |   20 ++++++++++----------
>  1 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
> index a0845dc..a02c43b 100644
> --- a/fs/cifs/cifssmb.c
> +++ b/fs/cifs/cifssmb.c
> @@ -88,29 +88,29 @@ static struct {
>   *	on failure - errno
>   */
>  static int
> -cifs_strncpy_to_host(char **dst, const char *src, const int maxlen,
> +cifs_strlcpy_to_host(char **dst, const char *src, const int maxlen,
>  		 const bool is_unicode, const struct nls_table *nls_codepage)
>  {
>  	int plen;
>  
>  	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;
> +			goto cifs_strlcpy_to_host_ErrExit;
>  		cifs_strfromUCS_le(*dst, (__le16 *)src, plen, nls_codepage);
> +		(*dst)[plen] = 0;
> +		(*dst)[plen+1] = 0; /* needed for Unicode */
>  	} else {
>  		plen = strnlen(src, maxlen);
>  		*dst = kmalloc(plen + 2, GFP_KERNEL);
>  		if (!*dst)
> -			goto cifs_strncpy_to_host_ErrExit;
> -		strncpy(*dst, src, plen);
> +			goto cifs_strlcpy_to_host_ErrExit;
> +		strlcpy(*dst, src, plen);
>  	}
> -	(*dst)[plen] = 0;
> -	(*dst)[plen+1] = 0; /* harmless for ASCII case, needed for Unicode */
>  	return 0;
>  
> -cifs_strncpy_to_host_ErrExit:
> +cifs_strlcpy_to_host_ErrExit:
>  	cERROR(1, ("Failed to allocate buffer for string\n"));
>  	return -ENOMEM;
>  }
> @@ -4029,7 +4029,7 @@ parse_DFS_referrals(TRANSACTION2_GET_DFS_REFER_RSP *pSMBr,
>  		/* copy DfsPath */
>  		temp = (char *)ref + le16_to_cpu(ref->DfsPathOffset);
>  		max_len = data_end - temp;
> -		rc = cifs_strncpy_to_host(&(node->path_name), temp,
> +		rc = cifs_strlcpy_to_host(&(node->path_name), temp,
>  					max_len, is_unicode, nls_codepage);
>  		if (rc)
>  			goto parse_DFS_referrals_exit;
> @@ -4037,7 +4037,7 @@ parse_DFS_referrals(TRANSACTION2_GET_DFS_REFER_RSP *pSMBr,
>  		/* copy link target UNC */
>  		temp = (char *)ref + le16_to_cpu(ref->NetworkAddressOffset);
>  		max_len = data_end - temp;
> -		rc = cifs_strncpy_to_host(&(node->node_name), temp,
> +		rc = cifs_strlcpy_to_host(&(node->node_name), temp,
>  					max_len, is_unicode, nls_codepage);
>  		if (rc)
>  			goto parse_DFS_referrals_exit;
> _______________________________________________
> linux-cifs-client mailing list
> [email protected]
> https://lists.samba.org/mailman/listinfo/linux-cifs-client
> 

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