A potential bug to squeeze extra memory through command line arguments

haris iqbal haris.phnx at gmail.com
Wed Feb 24 12:32:01 UTC 2016

On Fri, Feb 19, 2016 at 6:55 PM, haris iqbal <haris.phnx at gmail.com> wrote:
> On Fri, Feb 19, 2016 at 6:00 PM, Dmitry V. Levin <ldv at altlinux.org> wrote:
>> Hi,
>> On Fri, Feb 19, 2016 at 05:47:40PM +0530, haris iqbal wrote:
>>> Hello.
>>> I was going through the code of strace.c, when I found this line in
>>> init() function.
>>> tcbtabsize = argc;  /* Surely enough for all -p args.  */
>>> That set me thinking is it really is what I think it is. Can it really
>>> be that strace will allocate that many tcbtab pointers and tcp
>>> structures as many arguments I give to the program.
>> The fun of the situation is that after commit v4.6-258-ge8172b7
>> this is no longer enough for all -p args.
>>> So I set out to find out whether there is a way to make strace take a
>>> lot of memory, without actually using it.
>>> Firstly I found out that if there is a way to give large number of
>>> arguments and still make strace work as it should. It turns out that
>>> there is. One can give as many -i (or some other such arguments) as
>>> arguments and the strace would work properly.
>>> Then, I wanted to make sure whether the code is actually allocating
>>> that many resources as as there are arguments. So I went into the
>>> cleanup() function that is called through error_msg_and_die(). And
>>> there I added debugging code to see how many of them were actually
>>> allocated. It turns out that they were allocated.
>>> I added a print statement in the for loop of cleanup(). It ran for as
>>> many number of times as there were arguments. Although the usefull one
>>> was the first one only, which had the pid of the process being traced
>>> (strace was run with -p <pid> option).
>>> So, summing it all up. I ran a script which gave strace a -p option
>>> and a process pid to trace. Along with that some 600000 -i options. It
>>> ran successfully with a memory consumption of around 100mb. Thats a
>>> lot for a small system with limited memory.
>> OK, so what would you suggest to change in initial tcb table allocation?
> There are still many portions of code which I have not understood
> properly, but I am to make a suggestion with whatever knowledge I have
> of the code. I would say to postpone the allocation till alloctcb() is
> called. But I can see many places where alloctcb() is called is by
> checking whether tcp->pid is present or not. And for that check, one
> would need an allocated structure with a 0 value.
> Are we allocating tcb table for multiple -p options only or does it
> have some other purpose?

Ok. I have come up with a separate memory model for tcbtab. In this
model, we will use a linked list instead of a global array of pointers

The structure

struct s_tcbtab
    struct tcb* data;
    struct s_tcbtab* next;

And a global head of the linked list.

struct s_tcbtab* head_tcbtab = NULL;

Now, no need for the following code in init()

    /* Allocate the initial tcbtab.  */
    tcbtabsize = argc;  /* Surely enough for all -p args.  */

    tcbtab = xcalloc(tcbtabsize, sizeof(tcbtab[0]));
    tcp = xcalloc(tcbtabsize, sizeof(*tcp));
    for (tcbi = 0; tcbi < tcbtabsize; tcbi++)
        tcbtab[tcbi] = tcp++;

tcbtabsize will be 0.

The changes will be made in the alloctab(). The code that will be added is

if(head_tcbtab == NULL)
    // for the first allocation
    head_tcbtab = malloc(sizeof(s_tcbtab));
    head_tcbtab->next = NULL
    tcp = head_tcbtab -> data = malloc(sizeof(struct tcb));
    // further adding tabs
    temp_tcb = head_tcbtab;
    while(temp_tcb->next != NULL)
        temp_tcb = temp_tcb->next;

    temp_tcb->next = malloc(sizeof(s_tcbtab));
    temp_tcb = temp_tcb->next;

    temp_tcb->next = NULL;
    tcp = temp_tcb->data = malloc(sizeof(struct tcb));

after this, the code can continue with the normal

    memset(tcp, 0, sizeof(*tcp));
    tcp->pid = pid;
    tcp->currpers = current_personality;
    return tcp;

Additional code has to be changed at places where there is a for loop
iteration like in startup_attach()

we have to change this

    for (tcbi = 0; tcbi < tcbtabsize; tcbi++)
        tcp = tcbtab[tcbi];


    tcbi = head_tcbtab;
    while(tcbi != NULL)
        tcp = tcbi->data;

>> --
>> ldv
>> ------------------------------------------------------------------------------
>> Site24x7 APM Insight: Get Deep Visibility into Application Performance
>> APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month
>> Monitor end-to-end web transactions and take corrective actions now
>> Troubleshoot faster and improve end-user experience. Signup Now!
>> http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140
>> _______________________________________________
>> Strace-devel mailing list
>> Strace-devel at lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/strace-devel
> --
> With regards,
> Md Haris Iqbal,
> Placement Coordinator, MTech IT
> NITK Surathkal,
> Contact: +91 8861996962


With regards,

Md Haris Iqbal,
Placement Coordinator, MTech IT
NITK Surathkal,
Contact: +91 8861996962

More information about the Strace-devel mailing list