[PATCH nptl] Fix memory overwrite bug in pthread_attr_getaffinity().

[Chris Metcalf]

On 1/11/2010 12:47 PM, Carmelo AMOROSO wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Chris Metcalf wrote:
>   
>> If the caller requests fewer bytes of cpu_set_t data than are
>> available from the system, the code will still copy all of the
>> system's data to the user, overwriting additional memory.
>>
>> Signed-off-by: Chris Metcalf <cmetcalf at tilera.com>
>> ---
>>  .../unix/sysv/linux/pthread_attr_getaffinity.c     |    8 +++++++-
>>  1 files changed, 7 insertions(+), 1 deletions(-)
>>
>> diff --git a/libpthread/nptl/sysdeps/unix/sysv/linux/pthread_attr_getaffinity.c b/libpthread/nptl/sysdeps/unix/sysv/linux/pthread_attr_getaffinity.c
>> index 5a3d418..376eac8 100644
>> --- a/libpthread/nptl/sysdeps/unix/sysv/linux/pthread_attr_getaffinity.c
>> +++ b/libpthread/nptl/sysdeps/unix/sysv/linux/pthread_attr_getaffinity.c
>> @@ -43,9 +43,15 @@ __pthread_attr_getaffinity_new (const pthread_attr_t *attr, size_t cpusetsize,
>>  	if (((char *) iattr->cpuset)[cnt] != 0)
>>  	  return EINVAL;
>>  
>> -      void *p = mempcpy (cpuset, iattr->cpuset, iattr->cpusetsize);
>>        if (cpusetsize > iattr->cpusetsize)
>> +      {
>> +	void *p = mempcpy (cpuset, iattr->cpuset, iattr->cpusetsize);
>>  	memset (p, '\0', cpusetsize - iattr->cpusetsize);
>> +      }
>> +      else
>> +      {
>> +	memcpy (cpuset, iattr->cpuset, cpusetsize);
>> +      }
>>      }
>>    else
>>      /* We have no information.  */
>>     
>
> Chris,
> the patch is fine to me, anyway I've sent a slightly modified version to glibc list
> (as we agreed by emails) to raise the issue to them too: at the end I'd like to avoid
> diverging from glibc/nptl code from which we taken the nptl implementation.
> I'll just wait any feedback from Drepper & glibc community, to see if we can keep the same
> code, otherwise we can commit your patch into uClibc.
>
> Anyway I'm wondering how this exploit this issue ? do you have a real case in which this
> occurred ? or you were just doing a static code analysis and find it out ?
>   

test/nptl/tst-attr3.c seg faulted in our environment.  I didn't drill
down to why the cpusetsize that was passed (as sizeof(cpu_set_t))
differed from the iattr->cpusetsize, I just saw the mempcpy() overwrite
the link register on the stack and cause the function to try to branch
to PC=0 on return.

-- 
Chris Metcalf, Tilera Corp.
http://www.tilera.com