netdev - RE: [PATCH] rhashtable: Better error message on allocation failure

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]

Message-ID: <bc3b04ef968647a789431f482cc8246f@AcuMS.aculab.com>
Date: Wed, 29 Nov 2023 09:20:48 +0000
From: David Laight <David.Laight@...LAB.COM>
To: 'Jakub Kicinski' <kuba@...nel.org>, Kent Overstreet
	<kent.overstreet@...ux.dev>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>, Thomas Graf
	<tgraf@...g.ch>, Herbert Xu <herbert@...dor.apana.org.au>
Subject: RE: [PATCH] rhashtable: Better error message on allocation failure

From: Jakub Kicinski
> Sent: 29 November 2023 02:15
> 
> On Tue, 28 Nov 2023 20:57:05 -0500 Kent Overstreet wrote:
> > > Yes, that's problematic :(
> > > Let's leave out the GFP_NOWARN and add a pr_warn() instead of
> > > the WARN()?
> >
> > pr_warn() instead of WARN() is fine, but the stack trace from
> > warn_alloc() will be entirely useless.
> >
> > Perhaps if we had a GFP flag to just suppress the backtrace in
> > warn_alloc() - we could even stash a backtrace in the rhashtable at
> > rhashtable_init() time, if we want to print out a more useful one.
> 
> Interesting idea, up to you how far down the rabbit hole you're
> willing to go, really :)
> 
> Stating the obvious but would be good to add to the commit message,
> if you decide to implement this, how many rht instances there are
> on a sample system, IOW how much memory we expect the stacks to burn.

It's not really memory, just 'junk' in the console buffer.
But completely supressing the traceback from warn_alloc() (et al)
would give absolutely no indication of what failed.
You wouldn't even know it was a call from rhashtable().

Actually I'd have thought the traceback would show where the hash table
was being allocated - which would (mostly) tell you which one.
(Unless they were being allocated by a worker thread - which seems unlikely.)

IIRC the traceback includes the cpu registers (and code??) they probably
are noise for 'controlled' errors like kmalloc failing.

Is the whole extra trace really worthwhile at all?
How often does kmalloc() really fail?
Although if the code recovers by using a smaller table then maybe
that might be worth a trace instead of the one from kmalloc().

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)