|
From: Dan S. <dan...@go...> - 2012-05-31 18:45:30
|
This bug report just came through the AT&T AST/ksh93 development list.
Can someone confirm that this is a bug, please?
Dan
---------- Forwarded message ----------
From: Glenn Fowler <gs...@re...>
Date: 31 May 2012 18:40
Subject: [ast-developers] Re: valgrind "exp-sgcheck" hit in
ast-ksh.2012-05-18's pathtemp.c:322 ...
To: ast...@re..., rol...@nr...
here is a test using <stdio.h> that illustrates a valgrind bug
that I think is related to the one we are seeing in libast::pathtemp()
it looks like valgrind may be getting confused by printf varargs
--
t.c
--
#include <stdio.h>
#include <string.h>
int main(int argc, char** argv)
{
char a[16];
char b[16];
strcpy(a, "0123456789abcd");
strcpy(b, "01.01");
printf("%s => %s\n", a, b);
return 0;
}
--
valgrind command and output
--
valgrind --tool=exp-sgcheck --trace-children=yes --read-var-info=yes
--num-callers=50 a.out
==24375== exp-sgcheck, a stack and global array overrun detector
==24375== NOTE: This is an Experimental-Class Valgrind Tool
==24375== Copyright (C) 2003-2011, and GNU GPL'd, by OpenWorks Ltd et al.
==24375== Using Valgrind-3.7.0 and LibVEX; rerun with -h for copyright info
==24375== Command: a.out
==24375==
==24375== Invalid read of size 1
==24375== at 0x4E6EF54: vfprintf (in /lib64/libc-2.11.3.so)
==24375== by 0x4E76329: printf (in /lib64/libc-2.11.3.so)
==24375== by 0x4005E1: main (t.c:11)
==24375== Address 0x7fefffda0 expected vs actual:
==24375== Expected: stack array "a" of size 16 in frame 2 back from here
==24375== Actual: stack array "b" of size 16 in frame 2 back from here
==24375== Actual: is 16 before Expected
==24375==
0123456789abcd => 01.01
==24375==
==24375== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)
_______________________________________________
ast-developers mailing list
ast...@re...
https://mailman.research.att.com/mailman/listinfo/ast-developers
|
|
From: Tom H. <to...@co...> - 2012-05-31 19:27:46
|
On 31/05/12 19:45, Dan Shelton wrote:
> This bug report just came through the AT&T AST/ksh93 development list.
> Can someone confirm that this is a bug, please?
Well it's not a bug as such more a limitation of the way that
exp-sgcheck works - there's a reason it's experimental.
Basically exp-sgcheck assumes that once a pointer has been used to
access one array it will always access the same array until the end of
the current function.
I believe that what you are seeing is vfprintf violating that assumption
by using the same variable for each %s it expands which then causes
exp-sgcheck to warn when it sees it being used to access the second array.
Tom
> Dan
>
> ---------- Forwarded message ----------
> From: Glenn Fowler<gs...@re...>
> Date: 31 May 2012 18:40
> Subject: [ast-developers] Re: valgrind "exp-sgcheck" hit in
> ast-ksh.2012-05-18's pathtemp.c:322 ...
> To: ast...@re..., rol...@nr...
>
>
>
>
> here is a test using<stdio.h> that illustrates a valgrind bug
> that I think is related to the one we are seeing in libast::pathtemp()
> it looks like valgrind may be getting confused by printf varargs
>
> --
> t.c
> --
> #include<stdio.h>
> #include<string.h>
>
> int main(int argc, char** argv)
> {
> char a[16];
> char b[16];
>
> strcpy(a, "0123456789abcd");
> strcpy(b, "01.01");
> printf("%s => %s\n", a, b);
> return 0;
> }
>
> --
> valgrind command and output
> --
> valgrind --tool=exp-sgcheck --trace-children=yes --read-var-info=yes
> --num-callers=50 a.out
> ==24375== exp-sgcheck, a stack and global array overrun detector
> ==24375== NOTE: This is an Experimental-Class Valgrind Tool
> ==24375== Copyright (C) 2003-2011, and GNU GPL'd, by OpenWorks Ltd et al.
> ==24375== Using Valgrind-3.7.0 and LibVEX; rerun with -h for copyright info
> ==24375== Command: a.out
> ==24375==
> ==24375== Invalid read of size 1
> ==24375== at 0x4E6EF54: vfprintf (in /lib64/libc-2.11.3.so)
> ==24375== by 0x4E76329: printf (in /lib64/libc-2.11.3.so)
> ==24375== by 0x4005E1: main (t.c:11)
> ==24375== Address 0x7fefffda0 expected vs actual:
> ==24375== Expected: stack array "a" of size 16 in frame 2 back from here
> ==24375== Actual: stack array "b" of size 16 in frame 2 back from here
> ==24375== Actual: is 16 before Expected
> ==24375==
> 0123456789abcd => 01.01
> ==24375==
> ==24375== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)
>
> _______________________________________________
> ast-developers mailing list
> ast...@re...
> https://mailman.research.att.com/mailman/listinfo/ast-developers
>
> ------------------------------------------------------------------------------
> Live Security Virtual Conference
> Exclusive live event will cover all the ways today's security and
> threat landscape has changed and how IT managers can respond. Discussions
> will include endpoint security, mobile security and the latest in malware
> threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
> _______________________________________________
> Valgrind-developers mailing list
> Val...@li...
> https://lists.sourceforge.net/lists/listinfo/valgrind-developers
--
Tom Hughes (to...@co...)
http://compton.nu/
|
|
From: Dan S. <dan...@go...> - 2012-06-08 13:51:18
|
On 31 May 2012 21:27, Tom Hughes <to...@co...> wrote: > On 31/05/12 19:45, Dan Shelton wrote: > >> This bug report just came through the AT&T AST/ksh93 development list. >> Can someone confirm that this is a bug, please? > > > Well it's not a bug as such more a limitation of the way that exp-sgcheck > works - there's a reason it's experimental. > > Basically exp-sgcheck assumes that once a pointer has been used to access > one array it will always access the same array until the end of the current > function. Is it possible to change that so that functions with variable number of arguments like sprintf work as expected? |
|
From: Dan S. <dan...@go...> - 2012-06-08 13:52:05
|
On 8 June 2012 15:51, Dan Shelton <dan...@go...> wrote: > On 31 May 2012 21:27, Tom Hughes <to...@co...> wrote: >> On 31/05/12 19:45, Dan Shelton wrote: >> >>> This bug report just came through the AT&T AST/ksh93 development list. >>> Can someone confirm that this is a bug, please? >> >> >> Well it's not a bug as such more a limitation of the way that exp-sgcheck >> works - there's a reason it's experimental. >> >> Basically exp-sgcheck assumes that once a pointer has been used to access >> one array it will always access the same array until the end of the current >> function. > > Is it possible to change that so that functions with variable number > of arguments like sprintf work as expected? (this should be implemented in a general way, not by whitelisting sprintf/printf/fprintf) |
|
From: Tom H. <to...@co...> - 2012-06-08 13:55:02
|
On 08/06/12 14:51, Dan Shelton wrote:
> On 31 May 2012 21:27, Tom Hughes <to...@co...> wrote:
>> On 31/05/12 19:45, Dan Shelton wrote:
>>
>>> This bug report just came through the AT&T AST/ksh93 development list.
>>> Can someone confirm that this is a bug, please?
>>
>>
>> Well it's not a bug as such more a limitation of the way that exp-sgcheck
>> works - there's a reason it's experimental.
>>
>> Basically exp-sgcheck assumes that once a pointer has been used to access
>> one array it will always access the same array until the end of the current
>> function.
>
> Is it possible to change that so that functions with variable number
> of arguments like sprintf work as expected?
It's nothing to do with having a variable number of arguments, you could
get exactly the same warning with a fixed number of arguments if you
wrote a function like this:
void foo(char *a, char *b)
{
char *p;
for (p = a; *p; p++)
{
...
}
for (p = b; *p; p++)
{
...
}
}
The problem is the reuse of a variable (p in this example) to access
different arrays.
And no, we know of no good solution.
Basically exp-sgcheck was an interesting experiment, but in my
experience it is not workable for real world programs.
Tom
--
Tom Hughes (to...@co...)
http://compton.nu/
|
|
From: Dan S. <dan...@go...> - 2012-06-08 14:08:38
|
On 8 June 2012 15:54, Tom Hughes <to...@co...> wrote:
> On 08/06/12 14:51, Dan Shelton wrote:
>>
>> On 31 May 2012 21:27, Tom Hughes <to...@co...> wrote:
>>>
>>> On 31/05/12 19:45, Dan Shelton wrote:
>>>
>>>> This bug report just came through the AT&T AST/ksh93 development list.
>>>> Can someone confirm that this is a bug, please?
>>>
>>>
>>>
>>> Well it's not a bug as such more a limitation of the way that exp-sgcheck
>>> works - there's a reason it's experimental.
>>>
>>> Basically exp-sgcheck assumes that once a pointer has been used to access
>>> one array it will always access the same array until the end of the
>>> current
>>> function.
>>
>>
>> Is it possible to change that so that functions with variable number
>> of arguments like sprintf work as expected?
>
>
> It's nothing to do with having a variable number of arguments, you could get
> exactly the same warning with a fixed number of arguments if you wrote a
> function like this:
>
> void foo(char *a, char *b)
> {
> char *p;
>
> for (p = a; *p; p++)
> {
> ...
> }
>
> for (p = b; *p; p++)
> {
> ...
> }
> }
>
> The problem is the reuse of a variable (p in this example) to access
> different arrays.
It can't get an option to always check the pointer for every access?
> And no, we know of no good solution.
>
> Basically exp-sgcheck was an interesting experiment, but in my experience it
> is not workable for real world programs.
Is there any tool based on valgrind which can do checks on global
arrays and stack variables and is not experimental?
|
|
From: Tom H. <to...@co...> - 2012-06-08 14:17:51
|
On 08/06/12 15:08, Dan Shelton wrote:
> On 8 June 2012 15:54, Tom Hughes <to...@co...> wrote:
>> On 08/06/12 14:51, Dan Shelton wrote:
>>>
>>> On 31 May 2012 21:27, Tom Hughes <to...@co...> wrote:
>>>>
>>>> On 31/05/12 19:45, Dan Shelton wrote:
>>>>
>>>>> This bug report just came through the AT&T AST/ksh93 development list.
>>>>> Can someone confirm that this is a bug, please?
>>>>
>>>>
>>>>
>>>> Well it's not a bug as such more a limitation of the way that exp-sgcheck
>>>> works - there's a reason it's experimental.
>>>>
>>>> Basically exp-sgcheck assumes that once a pointer has been used to access
>>>> one array it will always access the same array until the end of the
>>>> current
>>>> function.
>>>
>>>
>>> Is it possible to change that so that functions with variable number
>>> of arguments like sprintf work as expected?
>>
>>
>> It's nothing to do with having a variable number of arguments, you could get
>> exactly the same warning with a fixed number of arguments if you wrote a
>> function like this:
>>
>> void foo(char *a, char *b)
>> {
>> char *p;
>>
>> for (p = a; *p; p++)
>> {
>> ...
>> }
>>
>> for (p = b; *p; p++)
>> {
>> ...
>> }
>> }
>>
>> The problem is the reuse of a variable (p in this example) to access
>> different arrays.
>
> It can't get an option to always check the pointer for every access?
It does check it! That's the problem...
My example above actually might work, depending on exactly what the
compiler does, but it is easy to construct a variant that will fail.
The details of how sgcheck works can be found here:
http://valgrind.org/docs/manual/sg-manual.html#sg-manual.how-works.sg-checks
but basically, the first time an instruction is executed we remember
which array it accessed and then expect it to access the same array in
future, until the function ends anyway.
So if the compile realised those two loops were the same and merged them
that code would fail - a version which had two nested loops and used p
in the inner loop to point to different arrays selected by the outer
loop would always fail.
>> And no, we know of no good solution.
>>
>> Basically exp-sgcheck was an interesting experiment, but in my experience it
>> is not workable for real world programs.
>
> Is there any tool based on valgrind which can do checks on global
> arrays and stack variables and is not experimental?
No, because this is basically a research grade problem that is currently
(to my knowledge unsolved) as to how to do such checks on an unmodified
program.
Tools which can do so work at compile/link time by inserting guards
between the stack variables to spot overrunning accesses, or by
adjusting the code which accesses arrays to do bounds checcks. An
unmodified program will have arrays directly adjacent to each other in
memory making any attempt to spot such things very hard.
Tom
--
Tom Hughes (to...@co...)
http://compton.nu/
|
|
From: Dan S. <dan...@go...> - 2012-06-08 14:29:50
|
On 8 June 2012 16:17, Tom Hughes <to...@co...> wrote:
> On 08/06/12 15:08, Dan Shelton wrote:
>>
>> On 8 June 2012 15:54, Tom Hughes <to...@co...> wrote:
>>>
>>> On 08/06/12 14:51, Dan Shelton wrote:
>>>>
>>>>
>>>> On 31 May 2012 21:27, Tom Hughes <to...@co...> wrote:
>>>>>
>>>>>
>>>>> On 31/05/12 19:45, Dan Shelton wrote:
>>>>>
>>>>>> This bug report just came through the AT&T AST/ksh93 development list.
>>>>>> Can someone confirm that this is a bug, please?
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> Well it's not a bug as such more a limitation of the way that
>>>>> exp-sgcheck
>>>>> works - there's a reason it's experimental.
>>>>>
>>>>> Basically exp-sgcheck assumes that once a pointer has been used to
>>>>> access
>>>>> one array it will always access the same array until the end of the
>>>>> current
>>>>> function.
>>>>
>>>>
>>>>
>>>> Is it possible to change that so that functions with variable number
>>>> of arguments like sprintf work as expected?
>>>
>>>
>>>
>>> It's nothing to do with having a variable number of arguments, you could
>>> get
>>> exactly the same warning with a fixed number of arguments if you wrote a
>>> function like this:
>>>
>>> void foo(char *a, char *b)
>>> {
>>> char *p;
>>>
>>> for (p = a; *p; p++)
>>> {
>>> ...
>>> }
>>>
>>> for (p = b; *p; p++)
>>> {
>>> ...
>>> }
>>> }
>>>
>>> The problem is the reuse of a variable (p in this example) to access
>>> different arrays.
>>
>>
>> It can't get an option to always check the pointer for every access?
>
>
> It does check it! That's the problem...
>
> My example above actually might work, depending on exactly what the compiler
> does, but it is easy to construct a variant that will fail.
>
> The details of how sgcheck works can be found here:
>
> http://valgrind.org/docs/manual/sg-manual.html#sg-manual.how-works.sg-checks
>
> but basically, the first time an instruction is executed we remember which
> array it accessed and then expect it to access the same array in future,
> until the function ends anyway.
>
> So if the compile realised those two loops were the same and merged them
> that code would fail - a version which had two nested loops and used p in
> the inner loop to point to different arrays selected by the outer loop would
> always fail.
So basically you say: As soon as the compiler optimises we're doomed?
Can a different choice of compiler (pcc ?) or disabling the optimiser
help?
>>> And no, we know of no good solution.
>>>
>>> Basically exp-sgcheck was an interesting experiment, but in my experience
>>> it
>>> is not workable for real world programs.
>>
>>
>> Is there any tool based on valgrind which can do checks on global
>> arrays and stack variables and is not experimental?
>
>
> No, because this is basically a research grade problem that is currently (to
> my knowledge unsolved) as to how to do such checks on an unmodified program.
Why does Purify (your closest competitor AFAIK) handle this right?
|
|
From: Tom H. <to...@co...> - 2012-06-08 14:36:43
|
On 08/06/12 15:29, Dan Shelton wrote:
> On 8 June 2012 16:17, Tom Hughes <to...@co...> wrote:
>> On 08/06/12 15:08, Dan Shelton wrote:
>>>
>>> On 8 June 2012 15:54, Tom Hughes <to...@co...> wrote:
>>>>
>>>> On 08/06/12 14:51, Dan Shelton wrote:
>>>>>
>>>>>
>>>>> On 31 May 2012 21:27, Tom Hughes <to...@co...> wrote:
>>>>>>
>>>>>>
>>>>>> On 31/05/12 19:45, Dan Shelton wrote:
>>>>>>
>>>>>>> This bug report just came through the AT&T AST/ksh93 development list.
>>>>>>> Can someone confirm that this is a bug, please?
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> Well it's not a bug as such more a limitation of the way that
>>>>>> exp-sgcheck
>>>>>> works - there's a reason it's experimental.
>>>>>>
>>>>>> Basically exp-sgcheck assumes that once a pointer has been used to
>>>>>> access
>>>>>> one array it will always access the same array until the end of the
>>>>>> current
>>>>>> function.
>>>>>
>>>>>
>>>>>
>>>>> Is it possible to change that so that functions with variable number
>>>>> of arguments like sprintf work as expected?
>>>>
>>>>
>>>>
>>>> It's nothing to do with having a variable number of arguments, you could
>>>> get
>>>> exactly the same warning with a fixed number of arguments if you wrote a
>>>> function like this:
>>>>
>>>> void foo(char *a, char *b)
>>>> {
>>>> char *p;
>>>>
>>>> for (p = a; *p; p++)
>>>> {
>>>> ...
>>>> }
>>>>
>>>> for (p = b; *p; p++)
>>>> {
>>>> ...
>>>> }
>>>> }
>>>>
>>>> The problem is the reuse of a variable (p in this example) to access
>>>> different arrays.
>>>
>>>
>>> It can't get an option to always check the pointer for every access?
>>
>>
>> It does check it! That's the problem...
>>
>> My example above actually might work, depending on exactly what the compiler
>> does, but it is easy to construct a variant that will fail.
>>
>> The details of how sgcheck works can be found here:
>>
>> http://valgrind.org/docs/manual/sg-manual.html#sg-manual.how-works.sg-checks
>>
>> but basically, the first time an instruction is executed we remember which
>> array it accessed and then expect it to access the same array in future,
>> until the function ends anyway.
>>
>> So if the compile realised those two loops were the same and merged them
>> that code would fail - a version which had two nested loops and used p in
>> the inner loop to point to different arrays selected by the outer loop would
>> always fail.
>
> So basically you say: As soon as the compiler optimises we're doomed?
> Can a different choice of compiler (pcc ?) or disabling the optimiser
> help?
Disabling the optimiser might sometimes help - we generally recommend
using unoptimised code anyway.
Mostly though this will be triggered by code where even the high level
semantics cause a problem. An example in our code is a function with a
writev type interface like this:
void fn(const struct iovec *iov, int iovcnt)
{
int i;
for (i = 0; i < iovcnt; i++)
{
void *buffer = iov[i].iov_base;
int length = iov[i].iov_len;
... do stuff with buffer ...
}
}
because the body of that loop uses the same instructions to operate on
different arrays accessed by indexing iov you run into this problem.
>>>> And no, we know of no good solution.
>>>>
>>>> Basically exp-sgcheck was an interesting experiment, but in my experience
>>>> it
>>>> is not workable for real world programs.
>>>
>>>
>>> Is there any tool based on valgrind which can do checks on global
>>> arrays and stack variables and is not experimental?
>>
>>
>> No, because this is basically a research grade problem that is currently (to
>> my knowledge unsolved) as to how to do such checks on an unmodified program.
>
> Why does Purify (your closest competitor AFAIK) handle this right?
Because it works at link time to modify the code and insert canary words
between stack items I believe.
Tom
--
Tom Hughes (to...@co...)
http://compton.nu/
|
|
From: Philippe W. <phi...@sk...> - 2012-06-08 16:36:39
|
On Fri, 2012-06-08 at 15:36 +0100, Tom Hughes wrote:
> Mostly though this will be triggered by code where even the high level
> semantics cause a problem. An example in our code is a function with a
> writev type interface like this:
>
> void fn(const struct iovec *iov, int iovcnt)
> {
> int i;
>
> for (i = 0; i < iovcnt; i++)
> {
> void *buffer = iov[i].iov_base;
> int length = iov[i].iov_len;
>
> ... do stuff with buffer ...
> }
> }
>
> because the body of that loop uses the same instructions to operate on
> different arrays accessed by indexing iov you run into this problem.
(I am not very good at knowing what kind of IR instrumentation one can
reasonably do, so the below looks probably like science-fiction or
like 'here a miracle occurs during IR instrumentation').
Wouldn't it be possible to have the IR instrumented to make/remember
the difference between 'assign an address of a variable to buffer'
and 'increment/decrement the value of buffer' ?
Then the instrumented IR could say that a first access to an array can
only be valid if a value has been assigned.
A first access with a 'incremented/decremented value' would be
considered as invalid.
Philippe
|