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

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