On Tue, 2013-03-05 at 12:09 +0800, Chen Gang wrote:

于 2013年03月05日 11:37, Ben Hutchings 写道:

quoted

quoted

  I think what I have done is just like your choice "2."

quoted

  for me, I think they are equal:

-		strncpy(ctr.name, names[i], sizeof(ctr.name) - 1);
+		strlcpy(ctr.name, names[i], sizeof(ctr.name));


		strncpy(ctr.name, names[i], sizeof(ctr.name) - 1);
+ 		ctr.name[sizeof(ctr.name) - 1] = '\0';

They are not.  strncpy() pads with zeroes to the end of the given buffer
whereas strlcpy() adds only a single zero byte (and truncates if
necessary to fit the zero byte).

  ok, thank you. they are really not the same (originally, I did not
notice it)

  could you supply the reason:
    why need we zero all of ctr.name ?
    (for me, I think, keeping ctr.name just a zero-based string is ok)

This function calls rds_copy_info() to copy the whole of ctr into
userland.

If ctr is not completely initialised, then the values of the
uninitialised bytes are left over from the local variables of an earlier
system call.  If an attacker knows enough about the stack layout (easy
if this is a distribution kernel), they can make a series of system
calls that leak information about heap-allocated objects.  That can help
them to exploit other kernel bugs for privilege escalation.  So we
should initialise every bit of memory that is going to be copied to
userland.

(In fact, in general it's not even enough to initialise all fields of
the structure, because there may be padding bytes between them.  In this
case we know there isn't, because it's declared as packed.)

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.