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 _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
