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: <20140814123655.GT30401@n2100.arm.linux.org.uk> Date: Thu, 14 Aug 2014 13:36:55 +0100 From: Russell King - ARM Linux <linux@....linux.org.uk> To: Daniel Thompson <daniel.thompson@...aro.org> Cc: Anton Vorontsov <anton.vorontsov@...aro.org>, linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, kgdb-bugreport@...ts.sourceforge.net, patches@...aro.org, linaro-kernel@...ts.linaro.org, John Stultz <john.stultz@...aro.org>, Colin Cross <ccross@...roid.com>, kernel-team@...roid.com, Rob Herring <robherring2@...il.com>, Linus Walleij <linus.walleij@...aro.org>, Ben Dooks <ben.dooks@...ethink.co.uk>, Catalin Marinas <catalin.marinas@....com>, Dave Martin <Dave.Martin@....com>, Fabio Estevam <festevam@...il.com>, Frederic Weisbecker <fweisbec@...il.com>, Nicolas Pitre <nico@...aro.org> Subject: Re: [PATCH v8 4/4] ARM: Add KGDB/KDB FIQ debugger generic code On Thu, Aug 14, 2014 at 11:48:36AM +0100, Daniel Thompson wrote: > Don't worry, you certainly did make it clear! > > I actually went as far as removing the trace_* calls before having a > change of heart. As a result I spent some time looking at the > trace_hardirqs_on() code and, as far as I remember, I couldn't find a > lock in the code that wasn't bypassed when in_nmi() returned true. > > However the code is pretty complex and its certainly possible that I > missed something. Are you in a position to point any particular lock to > show I messed this up or are you working from memory? The complexity alone is an argument against calling it from FIQ context, because there's no guarantee that what's there will stay that way. trace_hardirqs_on() is made more complex because there's not one function with that name, but two. One is in the trace code, the other is in the lockdep code. Here's the trace code, which has at least two problems: trace_hardirqs_on -> stop_critical_timing -> check_critical_timing -> raw_spin_lock_irqsave(&max_trace_lock, flags); -> __trace_stack -> __ftrace_trace_stack -> save_stack_trace -> __save_stack_trace -> walk_stackframe -> unwind_frame -> unwind_find_idx -> spin_lock_irqsave(&unwind_lock, flags); > > So, how about moving the FIQ assembly code to entry-armv.S and making > > it less kgdb specific? (Though... we do want to keep a /very/ close > > eye on users to ensure that they don't do silly stuff with locking.) > > I think I can do that. > > Something like this? > > 1. Install the current trap handler code by default (either with or > without trace_* calls). Without trace_* calls. The FIQ should be transparent to tracing - tracing and lockdep is too much of an unknown (due to size and complexity) to ensure that it always follows the rules. > 2. Have default trap handler call an RCU notifier chain to allow it to > hook up with "normal" code without any hard coding (kgdb, IPI > handling, etc) Maybe... that sounds like it opens up FIQ for general purpose use which is something I want to avoid - I've little motivation to ensure that everyone plays by the rules. Given the choice, I'd rather maintain our present stance that using FIQs is hard and requires a lot of thought. > 3. Retain existing install_fiq() behaviour for people who want FIQ > to be a fast-interrupt rather than an NMI (for example, the > Raspberry pi USB optimizations). Yes, arch/arm/kernel/fiq.c should be relatively untouched by the core mods I suggested - we're just replacing the default "subs" instruction (which just ignores the FIQ) with something which can do something with it. It should be noted that using arch/arm/kernel/fiq.c would override this default FIQ handler - and that's a feature of it - fiq.c is based on the premise that there is only one single owner of the FIQ at any one time and there is no sharing of it, and that's something we want to retain. > 4. Ensure default handler is an exported symbol to allow the meths > drinkers from #3 to chain to logic for NMI/debug features > if they want to. That's not something I particularly want to permit - especially as the requirements for each "handler" are different - this new default code relies upon r13 pointing at some storage to save some registers, which may not be true of other FIQ handlers. Chaining it means that we force that use of r13 on others, and we then have to also come up with some way to export the correct r13 value. Given that fiq.c doesn't work with SMP, this isn't something I want to encourage. Further more, I can't get excited about Raspberry Pi "optimisations" using FIQ until I see the code, and I see no reason to think about catering for such stuff until the patches become visible here. -- FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up according to speedtest.net. -- 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/
Powered by blists - more mailing lists