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, 8 Nov 2021 13:22:47 +0100
From:   Marco Elver <elver@...gle.com>
To:     Dmitry Vyukov <dvyukov@...gle.com>
Cc:     paulmck@...nel.org, kasan-dev <kasan-dev@...glegroups.com>,
        Jun Miao <jun.miao@...driver.com>,
        Uladzislau Rezki <urezki@...il.com>,
        Josh Triplett <josh@...htriplett.org>,
        Steven Rostedt <rostedt@...dmis.org>,
        Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
        Lai Jiangshan <jiangshanlai@...il.com>,
        Joel Fernandes <joel@...lfernandes.org>,
        qiang.zhang1211@...il.com, RCU <rcu@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>, miaojun0823@....com,
        ryabinin.a.a@...il.com, Alexander Potapenko <glider@...gle.com>,
        jianwei.hu@...driver.com, melver@...gle.com
Subject: Re: [PATCH] rcu: avoid alloc_pages() when recording stack

On Mon, 8 Nov 2021 at 12:42, 'Dmitry Vyukov' via kasan-dev
<kasan-dev@...glegroups.com> wrote:
[...]
> > > > > > Reviewed-by: Uladzislau Rezki (Sony) <urezki@...il.com>
> > > > > I have queued it for review and testing, thank you both!  I do have
> > > > > some remaining concerns about this code being starved for memory.  I am
> > > > > wondering if the code needs to check the interrupt state.  And perhaps
> > > > > also whether locks are held.  I of course will refrain from sending
> > > > > this to mainline until these concerns are resolved.
> > > > >
> > > > > Marco, Dmitry, thoughts?

It's a general limitation of kasan_record_aux_stack_noalloc(), and if
stackdepot's pool is exhausted, we just don't record the stacktrace.
But given we just can't allocate any memory at all, I think it's the
best we can do.

However, given there are enough normal (with allocation allowed) uses
of stackdepot with KASAN enabled, the chances of stackdepot having
exhausted its pool when calling this are small. The condition when
recording the stack with the _noalloc() variant would fail is:
stackdepot runs out of space AND the same stack trace has not been
recorded before. And the only time we'd notice this is if we actually
hit a kernel bug that KASAN wants to report. The aggregate probability
of all this happening is very very low.

The original series has some more explanation:
https://lkml.kernel.org/r/20210913112609.2651084-1-elver@google.com

> > > > Well, the compiler does have an opinion:
> > > >
> > > > kernel/rcu/tree.c: In function ‘__call_rcu’:
> > > > kernel/rcu/tree.c:3029:2: error: implicit declaration of function ‘kasan_record_aux_stack_noalloc’; did you mean ‘kasan_record_aux_stack’? [-Werror=implicit-function-declaration]
> > > >   3029 |  kasan_record_aux_stack_noalloc(head);
> > > >        |  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > >        |  kasan_record_aux_stack
> > > >
> > > > I get the same message after merging in current mainline.
> > > >
> > > > I have therefore dropped this patch for the time being.
> > > >
> > > >                                                          Thanx, Paul
> > > Hi Paul E,
> > > The kasan_record_aux_stack_noalloc() is just introduce to linux-next now,
> > > and marking "Notice: this object is not reachable from any branch." in
> > > commit.
> > > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/include/linux/kasan.h?h=next-20211029&id=2f64acf6b653d01fbdc92a693f12bbf71a205926

"Notice: this object is not reachable from any branch." because it
kept changing the hash since it's in the -mm tree.

> > That would explain it!  Feel free to resend once the functionality is
> > more generally available.
>
> +kasan-dev@...glegroups.com mailing list
>
> I found the full commit with kasan_record_aux_stack_noalloc() implementation:
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20211029&id=2f64acf6b653d01fbdc92a693f12bbf71a205926
>
> but it calls kasan_save_stack() with second bool argument, and
> kasan_save_stack() accepts only 1 argument:
> https://elixir.bootlin.com/linux/latest/source/mm/kasan/common.c#L33
> so I am lost and can't comment on any of the Paul's questions re
> interrupts/spinlocks.

None of the kasan_record_aux_stack_noalloc() code and its dependencies
were in mainline yet. I think it landed a few days ago, but too late
for the patches here.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ