Re: [PATCH] Fixed out of bounds memory access

[Stephan Bergmann]

On 06/20/2012 10:22 PM, Caolán McNamara wrote:

I wonder if there would be any actual measurable performance benefit to
sticking in some hackery to keep the rtl_str_getLength symbol for
backwards compatibility but alias/define rtl_str_getLength to strlen in
order to get any benefit of those optimized strlens.


My gut assumption is that it wouldn't make too much of a difference in 
practice.  (Do we have large amounts of calls to rtl_str_getLength to 
begin with?)


And it would shoot down the valgrind workaround.  ;)

> Or do they only

kick in if the compiler can determine locally that the argument comes
from malloc ? dunno.


The actual optimization is likely (only) to read in complete words 
rather than individual bytes, which can be applied independently of the 
provenance of the argument.  (In fact, a compiler could even 
automatically optimize the implementation of rtl_str_getLength in that way.)


Stephan
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice

Re: [PATCH] Fixed out of bounds memory access

[Caolán McNamara]

On Wed, 2012-06-20 at 14:48 +0200, Stephan Bergmann wrote:
> rtl_str_getLength

I wonder if there would be any actual measurable performance benefit to
sticking in some hackery to keep the rtl_str_getLength symbol for
backwards compatibility but alias/define rtl_str_getLength to strlen in
order to get any benefit of those optimized strlens. Or do they only
kick in if the compiler can determine locally that the argument comes
from malloc ? dunno.

C.

___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice

Re: [PATCH] Fixed out of bounds memory access

[Stephan Bergmann]

On 06/18/2012 05:50 PM, Caolán McNamara wrote:

I bet this is one if the false-positive occasions where valgrind isn't
aware of one of the strlen performance hacks IIRC where glibc knows that
it can get away with traversing that strdup's memory block in 4byte
chunks in its strlen e.g. someone with a 4/8 character length username
wouldn't see it :-)

So... if you just changed the strlen to be aUser.getLength() + 1 would
that silence valgrind too ?


As it happened, I ran into this false valgrind warning now, too, and 
addressed it with 
 
"http://cgit.freedesktop.org/libreoffice/core/commit/?id=97beabccb73321a8d2e022705afa755f15e99fa0."; 
 (Argh, now that I re-read your above suggestion, using 
rtl::OString::getLength would really have been nicer than 
rtl_str_getLength.  Anyway...)


Stephan
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice

Re: [PATCH] Fixed out of bounds memory access

[Caolán McNamara]

On Mon, 2012-06-18 at 13:41 +0200, Stephan Bergmann wrote:
> In either case, the strdup(aUser.getStr()) should be OK?

I bet this is one if the false-positive occasions where valgrind isn't
aware of one of the strlen performance hacks IIRC where glibc knows that
it can get away with traversing that strdup's memory block in 4byte
chunks in its strlen e.g. someone with a 4/8 character length username
wouldn't see it :-)

So... if you just changed the strlen to be aUser.getLength() + 1 would
that silence valgrind too ?

C.

___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice

Re: [PATCH] Fixed out of bounds memory access

[Stephan Bergmann]

On 06/15/2012 05:45 PM, Marc-André Laverdière wrote:

Here is a patch for a small fish I caught while valgrinding. It was
accessing memory in the strdup.


Was it really?  My reading of the original


rtl::OUString aUserName;
rtl::OString aUser;
oslSecurity aSec = osl_getCurrentSecurity();
if( aSec )
{
osl_getUserName( aSec, &aUserName.pData );
aUser = rtl::OUStringToOString( aUserName, 
osl_getThreadTextEncoding() );
osl_freeSecurityHandle( aSec );
}

pSmProps[ 3 ].name  = const_cast(SmUserID);
pSmProps[ 3 ].type  = const_cast(SmARRAY8);
pSmProps[ 3 ].num_vals  = 1;
pSmProps[ 3 ].vals  = new SmPropValue;
pSmProps[ 3 ].vals->value   = strdup( aUser.getStr() );
pSmProps[ 3 ].vals->length  = strlen( (char *)pSmProps[ 3 ].vals->value 
)+1;


is that at the end aUser is either the empty string (if !aSec) or holds 
on to an OString copy of the data obtained from osl_getUserName.  In 
either case, the strdup(aUser.getStr()) should be OK?


Stephan
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice

[PATCH] Fixed out of bounds memory access

[Marc-André Laverdière]

Here is a patch for a small fish I caught while valgrinding. It was
accessing memory in the strdup.


Marc-André LAVERDIÈRE
"Perseverance must finish its work so that you may be mature and complete,
not lacking anything." -James 1:4
http://asimplediscipleslife.blogspot.com/
mlaverd.theunixplace.com


0001-Fixed-invalid-memory-access.patch
Description: Binary data
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice