linux-kernel - Re: [PATCH] trace: Add a free on close control mechanism for buffer_size

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <BANLkTin+WYW6Eahgn+yw2eTX44Erc5yBMg@mail.gmail.com>
Date:	Thu, 26 May 2011 11:00:43 -0700
From:	Vaibhav Nagarnaik <vnagarnaik@...gle.com>
To:	Steven Rostedt <rostedt@...dmis.org>
Cc:	Frederic Weisbecker <fweisbec@...il.com>,
	Ingo Molnar <mingo@...hat.com>, linux-kernel@...r.kernel.org,
	Michael Rubin <mrubin@...gle.com>,
	David Sharp <dhsharp@...gle.com>
Subject: Re: [PATCH] trace: Add a free on close control mechanism for buffer_size_kb

On Wed, May 25, 2011 at 8:29 PM, Steven Rostedt <rostedt@...dmis.org> wrote:
> On Fri, 2011-04-29 at 16:16 -0700, Vaibhav Nagarnaik wrote:
>
>> +static ssize_t tracing_resize_ring_buffer(unsigned long size)
>> +{
>> +     int cpu, ret = 0;
>> +
>> +     mutex_lock(&trace_types_lock);
>> +
>> +     tracing_stop();
>> +
>> +     /* disable all cpu buffers */
>> +     for_each_tracing_cpu(cpu) {
>> +             if (global_trace.data[cpu])
>> +                     atomic_inc(&global_trace.data[cpu]->disabled);
>> +             if (max_tr.data[cpu])
>> +                     atomic_inc(&max_tr.data[cpu]->disabled);
>> +     }
>> +
>> +     if (size != global_trace.entries)
>> +             ret = __tracing_resize_ring_buffer(size);
>> +
>> +     /* If check pages failed, return ENOMEM */
>> +     if (tracing_disabled)
>> +             ret = -ENOMEM;
>
> This is incorrect. tracing_disabled is set from
> __tracing_resize_ring_buffer() if the max buffer can't become the same
> size as the global buffer.

That's right. Thanks for catching it, I didn't see that as I was trying
to follow the logic in tracing_entries_write() function.

>
> What you should do is just check if ret is less than zero here.
>
>        if (ret < 0)
>                ret = -ENOMEM;

Instead, I don't have to check/set the return value. I can just return
"ret" to the caller.

>
> -- Steve
>
>
>> +
>> +     for_each_tracing_cpu(cpu) {
>> +             if (global_trace.data[cpu])
>> +                     atomic_dec(&global_trace.data[cpu]->disabled);
>> +             if (max_tr.data[cpu])
>> +                     atomic_dec(&max_tr.data[cpu]->disabled);
>> +     }
>> +
>> +     tracing_start();
>> +     mutex_unlock(&trace_types_lock);
>> +
>> +     return ret;
>> +}
>> +
>>
>
>


Vaibhav Nagarnaik
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/