[PATCH] staging: for dgrp, nd_ps_desc buffer length need be 'MAX_DESC_LEN + 1'

[Chen Gang]

On 2013年04月26日 15:43, Dan Carpenter wrote:
> On Wed, Apr 24, 2013 at 11:18:03AM +0800, Chen Gang wrote:
>> > 
>> > For dgrp, the buffer length of 'nd->nd_ps_desc' is 'MAX_DESC_LEN + 1', so
>> > the buffer length of 'getnode.nd_ps_desc' also need be 'MAX_DESC_LEN + 1',
>> > then can fully copy from 'nd->nd_ps_desc' to 'getnode.nd_ps_desc'.
>> > 
> This is a user space change so someone needs to check that it
> doesn't break anything.

Oh, really it is.

It seem the next fixing depends on the API definition between user mode
and kernel mode. For compatible for user mode, I prefer we do not touch
the API (although the API is intended, not defined explicitly).

If the API defines the 'nd_ps_desc' as "a NUL terminated string except
when the string length is MAX_DESC_LEN", the original is OK (I guess
the original 'nd->nd_ps_desc' use 'MAX_DESC_LEN + 1', so can be sure
the string always NUL terminated in kernel mode)

If the API defines the 'nd_ps_desc' must be "a NUL terminated string",
I prefer the fix below rather than my original one:

----------------------------diff begin----------------------------------

diff --git a/drivers/staging/dgrp/dgrp_dpa_ops.c b/drivers/staging/dgrp/dgrp_dpa_ops.c
index d99c227..f4b7f6f 100644
--- a/drivers/staging/dgrp/dgrp_dpa_ops.c
+++ b/drivers/staging/dgrp/dgrp_dpa_ops.c
@@ -391,7 +391,7 @@ static long dgrp_dpa_ioctl(struct file *file, unsigned int cmd,
 		getnode.nd_rx_byte = nd->nd_rx_byte;
 
 		memset(&getnode.nd_ps_desc, 0, MAX_DESC_LEN);
-		strncpy(getnode.nd_ps_desc, nd->nd_ps_desc, MAX_DESC_LEN);
+		strlcpy(getnode.nd_ps_desc, nd->nd_ps_desc, MAX_DESC_LEN);
 
 		if (copy_to_user(uarg, &getnode, sizeof(struct digi_node)))
 			return -EFAULT;

----------------------------diff end------------------------------------

Thanks.

-- 
Chen Gang

Asianux Corporation