[PATCH xts 1/2] libproto: Fix buffer read overrun

Peter Harris pharris at opentext.com
Wed Jan 21 13:03:17 PST 2015


On 2015-01-21 15:21, Ian Romanick wrote:
> On 01/20/2015 05:57 PM, Peter Harris wrote:
>> Found by -fsanitize=address
>>
>> Signed-off-by: Peter Harris <pharris at opentext.com>
>> ---
>>  xts5/src/libproto/ShowSup.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/xts5/src/libproto/ShowSup.c b/xts5/src/libproto/ShowSup.c
>> index a05ff7d..b8ba796 100644
>> --- a/xts5/src/libproto/ShowSup.c
>> +++ b/xts5/src/libproto/ShowSup.c
>> @@ -581,7 +581,7 @@ int	format;
>>  		int     i;
>>  		
>>  		if (nval > 0) {
>> -		    valuePtr = (CARD32 *) ((CARD32 *) rp + size);
>> +		    valuePtr = (CARD32 *) ((CARD8 *) rp + size);
> 
> The original code seems so bogus that the error must be trivially
> observable.  How did this remain undetected for so long?  It was in the
> initial import in February 2005... 10 years ago!

The values are thrown away unless you set XT_DEBUG >= 2 (the default is
0), so the fact that it's trying to print garbage isn't likely to be
noticed.

You really need -fsanitize=address (or valgrind or efence or similar) to
trap on the bogus memory read. I never would have noticed if I hadn't
been playing with -fsanitize=address to try to find the bug fixed by
patch 2/2. (it didn't help; -fsanitize=address changed xtest enough that
it stopped triggering the bug in my server)

> As a side note... I'm impressed that ajax hasn't kill every bit of
> pre-C89 code from git.freedesktop.org. :)

:)

Peter Harris
-- 
               Open Text Connectivity Solutions Group
Peter Harris                    http://connectivity.opentext.com/
Research and Development        Phone: +1 905 762 6001
pharris at opentext.com            Toll Free: 1 877 359 4866


More information about the xorg-devel mailing list