public inbox for trinity@vger.kernel.org
 help / color / mirror / Atom feed
* stack smash detected bug
@ 2013-10-04  1:53 Ildar Muslukhov
  2013-10-04 15:29 ` Dave Jones
  0 siblings, 1 reply; 6+ messages in thread
From: Ildar Muslukhov @ 2013-10-04  1:53 UTC (permalink / raw)
  To: Dave Jones, trinity

Hi,

I've been looking through the strange behavior today, where I am
getting lots of "stack smashing detected" and found that the most
probable place is the mkcall function. Here is the call stack:
[0x440545] (stack check related calls)
[0x44050e] (stack check related calls)
[0x408db4]<-stack canary check
[0x412709]<-call mkcall
[0x402228]
[0x405586]
[0x40185a]
[0x412b44]
[0x401db1]<-main()

After looking into the code in mkcall:
>long mkcall(int childno)
>{
>        unsigned long olda1, olda2, olda3, olda4, olda5, olda6;
>        unsigned int call = shm->syscallno[childno];
>        unsigned long ret = 0;
>        int errno_saved;
>        char string[512], *sptr;
...
I suspect that string[512] is the issue. The simple tests confirms
that assumption (I've just commented out the block of color_arg
function calls that fill the buffer with parameter values).

Will provide a patch for that tomorrow.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: stack smash detected bug
  2013-10-04  1:53 stack smash detected bug Ildar Muslukhov
@ 2013-10-04 15:29 ` Dave Jones
  2013-10-04 17:11   ` Ildar Muslukhov
  0 siblings, 1 reply; 6+ messages in thread
From: Dave Jones @ 2013-10-04 15:29 UTC (permalink / raw)
  To: Ildar Muslukhov; +Cc: trinity

On Thu, Oct 03, 2013 at 06:53:08PM -0700, Ildar Muslukhov wrote:
 > Hi,
 > 
 > I've been looking through the strange behavior today, where I am
 > getting lots of "stack smashing detected" and found that the most
 > probable place is the mkcall function. Here is the call stack:
 > [0x440545] (stack check related calls)
 > [0x44050e] (stack check related calls)
 > [0x408db4]<-stack canary check
 > [0x412709]<-call mkcall
 > [0x402228]
 > [0x405586]
 > [0x40185a]
 > [0x412b44]
 > [0x401db1]<-main()
 > 
 > After looking into the code in mkcall:
 > >long mkcall(int childno)
 > >{
 > >        unsigned long olda1, olda2, olda3, olda4, olda5, olda6;
 > >        unsigned int call = shm->syscallno[childno];
 > >        unsigned long ret = 0;
 > >        int errno_saved;
 > >        char string[512], *sptr;
 > ...
 > I suspect that string[512] is the issue. The simple tests confirms
 > that assumption (I've just commented out the block of color_arg
 > function calls that fill the buffer with parameter values).

If that's getting overrun, I'm really curious what the string is,
because that should only be holding a single line of text.
Even with all the ansi codes it should be plenty.

	Dave

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: stack smash detected bug
  2013-10-04 15:29 ` Dave Jones
@ 2013-10-04 17:11   ` Ildar Muslukhov
  2013-10-04 17:17     ` Dave Jones
  0 siblings, 1 reply; 6+ messages in thread
From: Ildar Muslukhov @ 2013-10-04 17:11 UTC (permalink / raw)
  To: Dave Jones, trinity

Here is an example that causes the crash, it is obviously longer that 512 ascii:
[child0:1607] [31] utimensat(dfd=^[[1;36m497
, filename=^[[1;36m".//proc/4/task/4/cpusetd%s%d%d%d%d%s%s%s%d%d%s%d%d%d%d%s%s%d
%s%s%d%s%d%s%s%d%d%s%d%d%d%d%s%d%d%s%s%s%d%s%d%d%s%d%d%s%d%s%d%s%d%d%d%s%s%s%s%s
%d%s%s%d%s%d%d%d%d%d%s%d%s%s%d%s%d%d%d%d%d%d%s%d%d%s%s%s%d%d%d%d%d%d%s%s%d%s%s%d
%s%s%s%s%d%s%d%d%d%d%d%d%s%s%d%s%d%d%s%d%d%s%s%d%s%d%d%d%s%s%d%s%d%s%d%s%s%d%s%s
%d%d%s%s%s%s%s%d%s%d%d%d%s%s%d%s%s%d%s%s%d%s%d%d%s%d%s%d%d%s%s%d%s%d%d%d%s%s%d%s
%s%s%s%d%d%s%s%d%d%d%s%d%d%s%d%s%s%d%s%s%s%d%s%d%d%s%d%s%d%s%d%s%d%s%d%d%s%s%d%s
%d%s%s%d%d%s%s%s%s%d%d%d%d%d%d%d%s%s%s%d%d%d%s%d%s%s%d%d%s%s%s%s%d%d%s%s%d%d%d%s
%d%s%d%d%s%d%d%d%s%s%d%s%s%d%s%s%s%d%s%d%s%s%s%s%s%d%d%s%d%s%s%d%d%s%d%s%s%d%s%s
%d%d%d%s%d%d%d%s%d%d%d%s%s%s%d%d%d%s%d%d%d%s%s%d%d%s%s%s%d%d%d%d%d%s%s%d%s%d%d%s
%d%d%d%s%s%d%s%s%s%s%d%s%d%s%s%d%d%d%d%d%s%d%d%s%s%d%d%d%d%d%s%d%d%s%s%s%d%d%s%s
%d%s%s%d%d%d%s%d%s%s%s%d%s%s%s%s%s%s%s%s%s%s%s%s%d%d%s%d%s%d%d%s%s%s%s%s%s%d%s%d
%s%d%d%s%d%d%d%s%s%s%s%s%d%d%d%s%s%s%s%d%s%s%s%d%d%d%s%d%s%s%d%d%s%s%s%s%s%s%s%d
%s%d%s%s%d%d%d%s%s%d%s%s%s%d%s%d%s%
[child1:1608] [0] setreuid(ruid=0x400000000000000,
euid=0xffffffffffffffff) [child1:1608] = -1 (Operation not permitted)

On Fri, Oct 4, 2013 at 8:29 AM, Dave Jones <davej@redhat.com> wrote:
> On Thu, Oct 03, 2013 at 06:53:08PM -0700, Ildar Muslukhov wrote:
>  > Hi,
>  >
>  > I've been looking through the strange behavior today, where I am
>  > getting lots of "stack smashing detected" and found that the most
>  > probable place is the mkcall function. Here is the call stack:
>  > [0x440545] (stack check related calls)
>  > [0x44050e] (stack check related calls)
>  > [0x408db4]<-stack canary check
>  > [0x412709]<-call mkcall
>  > [0x402228]
>  > [0x405586]
>  > [0x40185a]
>  > [0x412b44]
>  > [0x401db1]<-main()
>  >
>  > After looking into the code in mkcall:
>  > >long mkcall(int childno)
>  > >{
>  > >        unsigned long olda1, olda2, olda3, olda4, olda5, olda6;
>  > >        unsigned int call = shm->syscallno[childno];
>  > >        unsigned long ret = 0;
>  > >        int errno_saved;
>  > >        char string[512], *sptr;
>  > ...
>  > I suspect that string[512] is the issue. The simple tests confirms
>  > that assumption (I've just commented out the block of color_arg
>  > function calls that fill the buffer with parameter values).
>
> If that's getting overrun, I'm really curious what the string is,
> because that should only be holding a single line of text.
> Even with all the ansi codes it should be plenty.
>
>         Dave
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: stack smash detected bug
  2013-10-04 17:11   ` Ildar Muslukhov
@ 2013-10-04 17:17     ` Dave Jones
  2013-10-04 17:30       ` Ildar Muslukhov
  0 siblings, 1 reply; 6+ messages in thread
From: Dave Jones @ 2013-10-04 17:17 UTC (permalink / raw)
  To: Ildar Muslukhov; +Cc: trinity

On Fri, Oct 04, 2013 at 10:11:04AM -0700, Ildar Muslukhov wrote:
 > Here is an example that causes the crash, it is obviously longer that 512 ascii:
 > [child0:1607] [31] utimensat(dfd=^[[1;36m497
 > , filename=^[[1;36m".//proc/4/task/4/cpusetd%s%d%d%d%d%s%s%s%d%d%s%d%d%d%d%s%s%d
 > %s%s%d%s%d%s%s%d%d%s%d%d%d%d%s%d%d%s%s%s%d%s%d%d%s%d%d%s%d%s%d%s%d%d%d%s%s%s%s%s
 > %d%s%s%d%s%d%d%d%d%d%s%d%s%s%d%s%d%d%d%d%d%d%s%d%d%s%s%s%d%d%d%d%d%d%s%s%d%s%s%d
 > %s%s%s%s%d%s%d%d%d%d%d%d%s%s%d%s%d%d%s%d%d%s%s%d%s%d%d%d%s%s%d%s%d%s%d%s%s%d%s%s
 > %d%d%s%s%s%s%s%d%s%d%d%d%s%s%d%s%s%d%s%s%d%s%d%d%s%d%s%d%d%s%s%d%s%d%d%d%s%s%d%s
 > %s%s%s%d%d%s%s%d%d%d%s%d%d%s%d%s%s%d%s%s%s%d%s%d%d%s%d%s%d%s%d%s%d%s%d%d%s%s%d%s
 > %d%s%s%d%d%s%s%s%s%d%d%d%d%d%d%d%s%s%s%d%d%d%s%d%s%s%d%d%s%s%s%s%d%d%s%s%d%d%d%s
 > %d%s%d%d%s%d%d%d%s%s%d%s%s%d%s%s%s%d%s%d%s%s%s%s%s%d%d%s%d%s%s%d%d%s%d%s%s%d%s%s
 > %d%d%d%s%d%d%d%s%d%d%d%s%s%s%d%d%d%s%d%d%d%s%s%d%d%s%s%s%d%d%d%d%d%s%s%d%s%d%d%s
 > %d%d%d%s%s%d%s%s%s%s%d%s%d%s%s%d%d%d%d%d%s%d%d%s%s%d%d%d%d%d%s%d%d%s%s%s%d%d%s%s
 > %d%s%s%d%d%d%s%d%s%s%s%d%s%s%s%s%s%s%s%s%s%s%s%s%d%d%s%d%s%d%d%s%s%s%s%s%s%d%s%d
 > %s%d%d%s%d%d%d%s%s%s%s%s%d%d%d%s%s%s%s%d%s%s%s%d%d%d%s%d%s%s%d%d%s%s%s%s%s%s%s%d
 > %s%d%s%s%d%d%d%s%s%d%s%s%s%d%s%d%s%
 > [child1:1608] [0] setreuid(ruid=0x400000000000000,
 > euid=0xffffffffffffffff) [child1:1608] = -1 (Operation not permitted)

Ah, of course.. the pathname mangler. Oops.

This perhaps ?

	Dave

diff --git a/syscall.c b/syscall.c
index 80f5a34..94d2f1b 100644
--- a/syscall.c
+++ b/syscall.c
@@ -222,9 +222,11 @@ long mkcall(int childno)
        unsigned int call = shm->syscallno[childno];
        unsigned long ret = 0;
        int errno_saved;
-       char string[512], *sptr;
+       char *string, *sptr;
        uid_t olduid = getuid();
 
+       string = malloc(page_size);
+
        shm->regenerate++;
 
        sptr = string;
@@ -318,6 +320,9 @@ skip_args:
 
        output(2, "%s\n", string);
 
+       free(string);
+
+
        if (dopause == TRUE)
                sleep(1);

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: stack smash detected bug
  2013-10-04 17:17     ` Dave Jones
@ 2013-10-04 17:30       ` Ildar Muslukhov
  2013-10-04 17:40         ` Dave Jones
  0 siblings, 1 reply; 6+ messages in thread
From: Ildar Muslukhov @ 2013-10-04 17:30 UTC (permalink / raw)
  To: Dave Jones; +Cc: trinity

I would rather avoid buffers completely and output directly, I am
planing to take care of it in my "log cleanup" patch that we discussed
before.

On Fri, Oct 4, 2013 at 10:17 AM, Dave Jones <davej@redhat.com> wrote:
> On Fri, Oct 04, 2013 at 10:11:04AM -0700, Ildar Muslukhov wrote:
>  > Here is an example that causes the crash, it is obviously longer that 512 ascii:
>  > [child0:1607] [31] utimensat(dfd=^[[1;36m497
>  > , filename=^[[1;36m".//proc/4/task/4/cpusetd%s%d%d%d%d%s%s%s%d%d%s%d%d%d%d%s%s%d
>  > %s%s%d%s%d%s%s%d%d%s%d%d%d%d%s%d%d%s%s%s%d%s%d%d%s%d%d%s%d%s%d%s%d%d%d%s%s%s%s%s
>  > %d%s%s%d%s%d%d%d%d%d%s%d%s%s%d%s%d%d%d%d%d%d%s%d%d%s%s%s%d%d%d%d%d%d%s%s%d%s%s%d
>  > %s%s%s%s%d%s%d%d%d%d%d%d%s%s%d%s%d%d%s%d%d%s%s%d%s%d%d%d%s%s%d%s%d%s%d%s%s%d%s%s
>  > %d%d%s%s%s%s%s%d%s%d%d%d%s%s%d%s%s%d%s%s%d%s%d%d%s%d%s%d%d%s%s%d%s%d%d%d%s%s%d%s
>  > %s%s%s%d%d%s%s%d%d%d%s%d%d%s%d%s%s%d%s%s%s%d%s%d%d%s%d%s%d%s%d%s%d%s%d%d%s%s%d%s
>  > %d%s%s%d%d%s%s%s%s%d%d%d%d%d%d%d%s%s%s%d%d%d%s%d%s%s%d%d%s%s%s%s%d%d%s%s%d%d%d%s
>  > %d%s%d%d%s%d%d%d%s%s%d%s%s%d%s%s%s%d%s%d%s%s%s%s%s%d%d%s%d%s%s%d%d%s%d%s%s%d%s%s
>  > %d%d%d%s%d%d%d%s%d%d%d%s%s%s%d%d%d%s%d%d%d%s%s%d%d%s%s%s%d%d%d%d%d%s%s%d%s%d%d%s
>  > %d%d%d%s%s%d%s%s%s%s%d%s%d%s%s%d%d%d%d%d%s%d%d%s%s%d%d%d%d%d%s%d%d%s%s%s%d%d%s%s
>  > %d%s%s%d%d%d%s%d%s%s%s%d%s%s%s%s%s%s%s%s%s%s%s%s%d%d%s%d%s%d%d%s%s%s%s%s%s%d%s%d
>  > %s%d%d%s%d%d%d%s%s%s%s%s%d%d%d%s%s%s%s%d%s%s%s%d%d%d%s%d%s%s%d%d%s%s%s%s%s%s%s%d
>  > %s%d%s%s%d%d%d%s%s%d%s%s%s%d%s%d%s%
>  > [child1:1608] [0] setreuid(ruid=0x400000000000000,
>  > euid=0xffffffffffffffff) [child1:1608] = -1 (Operation not permitted)
>
> Ah, of course.. the pathname mangler. Oops.
>
> This perhaps ?
>
>         Dave
>
> diff --git a/syscall.c b/syscall.c
> index 80f5a34..94d2f1b 100644
> --- a/syscall.c
> +++ b/syscall.c
> @@ -222,9 +222,11 @@ long mkcall(int childno)
>         unsigned int call = shm->syscallno[childno];
>         unsigned long ret = 0;
>         int errno_saved;
> -       char string[512], *sptr;
> +       char *string, *sptr;
>         uid_t olduid = getuid();
>
> +       string = malloc(page_size);
> +
>         shm->regenerate++;
>
>         sptr = string;
> @@ -318,6 +320,9 @@ skip_args:
>
>         output(2, "%s\n", string);
>
> +       free(string);
> +
> +
>         if (dopause == TRUE)
>                 sleep(1);
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: stack smash detected bug
  2013-10-04 17:30       ` Ildar Muslukhov
@ 2013-10-04 17:40         ` Dave Jones
  0 siblings, 0 replies; 6+ messages in thread
From: Dave Jones @ 2013-10-04 17:40 UTC (permalink / raw)
  To: Ildar Muslukhov; +Cc: trinity

On Fri, Oct 04, 2013 at 10:30:34AM -0700, Ildar Muslukhov wrote:
 > I would rather avoid buffers completely and output directly, I am
 > planing to take care of it in my "log cleanup" patch that we discussed
 > before.
 
Ok, I'll wait for that.

	Dave

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2013-10-04 17:40 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-04  1:53 stack smash detected bug Ildar Muslukhov
2013-10-04 15:29 ` Dave Jones
2013-10-04 17:11   ` Ildar Muslukhov
2013-10-04 17:17     ` Dave Jones
2013-10-04 17:30       ` Ildar Muslukhov
2013-10-04 17:40         ` Dave Jones

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox