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
| ||
|
Message-Id: <6D816814-1334-4F22-AFF8-B5E42254038E@gmail.com> Date: Tue, 19 Dec 2023 11:25:46 +0200 From: Martin Zaharinov <micron10@...il.com> To: Thomas Gleixner <tglx@...utronix.de> Cc: peterz@...radead.org, netdev <netdev@...r.kernel.org>, Paolo Abeni <pabeni@...hat.com>, patchwork-bot+netdevbpf@...nel.org, Jakub Kicinski <kuba@...nel.org>, Stephen Hemminger <stephen@...workplumber.org>, kuba+netdrv@...nel.org, dsahern@...il.com, Eric Dumazet <edumazet@...gle.com> Subject: Re: Urgent Bug Report Kernel crash 6.5.2 Hi Thomas, Thanks for your response! > On 12 Dec 2023, at 20:16, Thomas Gleixner <tglx@...utronix.de> wrote: > > Martin! > > On Sat, Dec 09 2023 at 01:01, Martin Zaharinov wrote: >>> On 9 Dec 2023, at 0:20, Thomas Gleixner <tglx@...utronix.de> wrote: >>> That's definitely not a RCU problem. It's a simple refcount fail. >>> >> Is this a problem or only simple fail , and is it possible to catch >> what is a problem and fix this fail. > > Underaccounting a reference count is potentially Use After Free. > > if (rcuref_put(ref)) > call_rcu(ref....); > > So after the grace period is over @ref will be freed. Depending on the > timing the context which does the extra put() might already operate on a > freed object. > > How to catch that, that's a good question. There is no instrumentation > so far for this. Below is a straight forward trace_printk() based > tracking of rcurefs, which should help to narrow down the context. > > Btw, how easy is this to reproduce? Its not easy this report is generate on machine with 5-6k users , with traffic and one time is show on 1 day , other show after 4-5 days… > > Thanks, > > tglx > --- > --- a/include/linux/rcuref.h > +++ b/include/linux/rcuref.h > @@ -64,8 +64,10 @@ static inline __must_check bool rcuref_g > * Unconditionally increase the reference count. The saturation and > * dead zones provide enough tolerance for this. > */ > - if (likely(!atomic_add_negative_relaxed(1, &ref->refcnt))) > + if (likely(!atomic_add_negative_relaxed(1, &ref->refcnt))) { > + trace_printk("get(FASTPATH): %px\n", ref); > return true; > + } > > /* Handle the cases inside the saturation and dead zones */ > return rcuref_get_slowpath(ref); > @@ -84,8 +86,10 @@ static __always_inline __must_check bool > * Unconditionally decrease the reference count. The saturation and > * dead zones provide enough tolerance for this. > */ > - if (likely(!atomic_add_negative_release(-1, &ref->refcnt))) > + if (likely(!atomic_add_negative_release(-1, &ref->refcnt))) { > + trace_printk("put(FASTPATH): %px\n", ref); > return false; > + } > > /* > * Handle the last reference drop and cases inside the saturation > --- a/lib/rcuref.c > +++ b/lib/rcuref.c > @@ -200,6 +200,7 @@ bool rcuref_get_slowpath(rcuref_t *ref) > */ > if (cnt >= RCUREF_RELEASED) { > atomic_set(&ref->refcnt, RCUREF_DEAD); > + trace_printk("get(DEAD): %px %pS\n", ref, __builtin_return_address(0)); > return false; > } > > @@ -211,8 +212,15 @@ bool rcuref_get_slowpath(rcuref_t *ref) > * object memory, but prevents the obvious reference count overflow > * damage. > */ > - if (WARN_ONCE(cnt > RCUREF_MAXREF, "rcuref saturated - leaking memory")) > + if (cnt > RCUREF_MAXREF) { > + trace_printk("get(SATURATED): %px %pS\n", ref, __builtin_return_address(0)); > + WARN_ONCE(1, "rcuref saturated - leaking memory"); > atomic_set(&ref->refcnt, RCUREF_SATURATED); > + } else { > + trace_printk("get(UNDEFINED): %px %pS\n", ref, __builtin_return_address(0)); > + WARN_ON_ONCE(1); > + } > + > return true; > } > EXPORT_SYMBOL_GPL(rcuref_get_slowpath); > @@ -248,9 +256,12 @@ bool rcuref_put_slowpath(rcuref_t *ref) > * require a retry. If this fails the caller is not > * allowed to deconstruct the object. > */ > - if (!atomic_try_cmpxchg_release(&ref->refcnt, &cnt, RCUREF_DEAD)) > + if (!atomic_try_cmpxchg_release(&ref->refcnt, &cnt, RCUREF_DEAD)) { > + trace_printk("put(NOTDEAD): %px %pS\n", ref, __builtin_return_address(0)); > return false; > + } > > + trace_printk("put(NOWDEAD): %px %pS\n", ref, __builtin_return_address(0)); > /* > * The caller can safely schedule the object for > * deconstruction. Provide acquire ordering. > @@ -264,7 +275,9 @@ bool rcuref_put_slowpath(rcuref_t *ref) > * put() operation is imbalanced. Warn, put the reference count back to > * DEAD and tell the caller to not deconstruct the object. > */ > - if (WARN_ONCE(cnt >= RCUREF_RELEASED, "rcuref - imbalanced put()")) { > + if (cnt >= RCUREF_RELEASED) { > + trace_printk("put(WASDEAD): %px %pS\n", ref, __builtin_return_address(0)); > + WARN_ONCE(1, "rcuref - imbalanced put()"); > atomic_set(&ref->refcnt, RCUREF_DEAD); > return false; > } > @@ -274,8 +287,13 @@ bool rcuref_put_slowpath(rcuref_t *ref) > * mean saturation value and tell the caller to not deconstruct the > * object. > */ > - if (cnt > RCUREF_MAXREF) > + if (cnt > RCUREF_MAXREF) { > + trace_printk("put(SATURATED): %px %pS\n", ref, __builtin_return_address(0)); > atomic_set(&ref->refcnt, RCUREF_SATURATED); > + } else { > + trace_printk("put(UNDEFINED): %px %pS\n", ref, __builtin_return_address(0)); > + WARN_ON_ONCE(1); > + } > return false; > } > EXPORT_SYMBOL_GPL(rcuref_put_slowpath); Apply this patch and will upload image on one machine as fast as possible and when get any reports will send you. Best regards, Martin
Powered by blists - more mailing lists