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

Chris Metcalf cmetcalf at tilera.com
Tue Jan 12 12:48:07 UTC 2010


On 1/12/2010 3:03 AM, Carmelo AMOROSO wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Chris Metcalf wrote:
>   
>> 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.
>>
>>     
>
> Thanks for the info... never failed on sh4 indeed. Anyway for you it should be
> useful to figure out the reason for the difference in the sizeof, likely it is hiding
> some problems some somewhere else.
>   

When I first realized what the problem was, I looked for other similar
uses of cpu_set_t and didn't see any.  So hopefully it was just this one
piece of code that was broken.

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




More information about the uClibc mailing list