linux-kernel - Re: [nsproxy] BUG: unable to handle kernel NULL pointer dereference at 0000000000000024

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] [day] [month] [year] [list]

Date:	Mon, 11 Mar 2013 14:12:08 +0600
From:	Rakib Mullick <rakib.mullick@...il.com>
To:	"Eric W. Biederman" <ebiederm@...ssion.com>
Cc:	Fengguang Wu <fengguang.wu@...el.com>, linux-kernel@...r.kernel.org
Subject: Re: [nsproxy] BUG: unable to handle kernel NULL pointer dereference
 at 0000000000000024

On Sat, Mar 9, 2013 at 10:48 PM, Rakib Mullick <rakib.mullick@...il.com> wrote:
> On Sat, Mar 9, 2013 at 2:33 PM, Eric W. Biederman <ebiederm@...ssion.com> wrote:
>> Rakib Mullick <rakib.mullick@...il.com> writes:
>>
>>> On Fri, Mar 8, 2013 at 10:01 PM, Eric W. Biederman
>>> <ebiederm@...ssion.com> wrote:
>>>>
>>>> When a new task is created one of two things needs to happen.
>>>> A) A reference count needs to be added to the current nsproxy.
>>>> B) B a new nsproxy needs to be created.
>>>>
>>>> The way that code works today is far from a shiny example of totally
>>>> clear code but it is not incorrect.
>>>>
>>>> By moving get_nsproxy down below the first return 0, you removed taking
>>>> the reference count in the one case it is important.
>>>>
>>>> Arguably we should apply the patch below for clarity, and I just might
>>>> queue it up for 3.10.
>>>>
>>> This one is much more cleaner. One thing regarding this patch, can we
>>> check the namespace related flags at copy_namespace() call time at
>>> copy_process(), also get_nsproxy()? I think this will reduce some
>>> extra function call overhead and as you've mentioned get_nsproxy() is
>>> needed at every process creation.
>>
>> If you can write a benchmark that can tell the difference, and the code
>> continues to be readable.  It would be worth making the change.
>>
>> My gut says you are proposing an optimization that won't have
>> a measurable impact.
>>
> Yes, it'll be hard to measure these sorts of optimization, though I'll
> try and will tell you if the impact is measurable :).
>
Well, it's quite certain that - the changes I've proposed that don't
have much noticable impact. From this tiny
changes we can't expect too much impact. The tinyest possible impact
could be on at every task creation path by
reducing some latency. So, to find it out I took the help of ftrace -
I used it's function_graph option with set_graph_function set to
do_fork.

So currently, at every task creation copy_namespaces() gets called and
it costs some like below (few snippet from ftrace log):

 3)   0.025 us    |        try_module_get();
 3) ! 887.805 us  |      } /* dup_mm */
 3)   0.278 us    |      copy_namespaces(); <------
 3)   0.094 us    |      copy_thread();

 3)   0.264 us    |      copy_namespaces(); <-----
 3)   0.091 us    |      copy_thread();

 3)   0.306 us    |      copy_namespaces();  <-----
 3)   0.086 us    |      copy_thread();

etc. copy_namespaces() just returns here.

On the otherhand, when namespace related flags are moved to
kernel/fork.c::copy_process(), copy_namespaces() never gets called
until flags are set.

So, this is what I did manage to get. If you're not convinced, then
you can drop it. But, we can't expect much than this for this simple
changes.

Thanks,
Rakib.
--
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/