[lkml]   [2010]   [Nov]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    Patch in this message
    SubjectRe: [PATCH v2 1/4] drivers: hwspinlock: add generic framework
    On Sat, Nov 27, 2010 at 12:53 AM, Russell King - ARM Linux
    <> wrote:
    > On Sat, Nov 27, 2010 at 12:18:55AM +0200, Ohad Ben-Cohen wrote:
    >> But then there's the other (quite reasonable) claim that says we
    >> shouldn't crash the machine because of a non fatal bug: if a crappy
    >> driver messes up, the user (not the developer) will most probably
    >> prefer the machine to keep running with degraded functionality rather
    >> than boot.
    > There's also the quite reasonable expectation that we shouldn't corrupt
    > user data.  With locking interfaces, if someone abuses them and they
    > fail to work, then the risk is data corruption due to races.  The safe
    > thing in that case is to panic - terminate that thread before it does
    > anything unsafe, thereby preventing data corruption.

    Makes sense.

    I considered hwspinlock as a peripheral which doesn't qualify to take
    down the system on failure, but in general I agree that there indeed
    might be critical user data involved. Especially if this is a
    framework and not just another driver.

    But as a framework that can be used on non ARM architectures as well,
    I do prefer to check for NULL pointers and not rely on the Oops.

    If we had a macro that would be compiled out on architectures that
    reliably produces an Oops on NULL dereference, but otherwise, would
    BUG_ON on them, that should satisfy everyone.

    The BUG_ON_MAPPABLE_NULL() macro below should achieve exactly that,
    please tell me what you think, thanks!

    arch/arm/include/asm/mman.h | 1 +
    include/asm-generic/bug.h | 40 ++++++++++++++++++++++++++++++++++++++++
    2 files changed, 41 insertions(+), 0 deletions(-)

    diff --git a/arch/arm/include/asm/mman.h b/arch/arm/include/asm/mman.h
    index 41f99c5..d4ee003 100644
    --- a/arch/arm/include/asm/mman.h
    +++ b/arch/arm/include/asm/mman.h
    @@ -1,4 +1,5 @@
    #include <asm-generic/mman.h>
    +#include <asm/pgtable.h>

    #define arch_mmap_check(addr, len, flags) \
    (((flags) & MAP_FIXED && (addr) < FIRST_USER_ADDRESS) ? -EINVAL : 0)
    diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
    index c2c9ba0..d211d9c 100644
    --- a/include/asm-generic/bug.h
    +++ b/include/asm-generic/bug.h
    @@ -2,6 +2,11 @@
    #define _ASM_GENERIC_BUG_H

    #include <linux/compiler.h>
    +#include <asm/mman.h>
    +#ifndef arch_mmap_check
    +#define arch_mmap_check(addr, len, flags) (0)

    #ifdef CONFIG_BUG

    @@ -53,6 +58,41 @@ struct bug_entry {
    #define BUG_ON(condition) do { if (unlikely(condition)) BUG(); } while(0)

    + * BUG_ON_MAPPABLE_NULL() - panic on NULL only if address 0 is mappable
    + * @addr: address to check
    + *
    + * In general, NULL dereference Oopses are not desirable, since they take down
    + * the system with them and make the user extremely unhappy. So as a general
    + * rule kernel code should avoid dereferencing NULL pointers by doing a
    + * simple check (when appropriate), and if needed, continue operating
    + * with reduced functionality rather than crash.
    + *
    + * _Critical_ kernel code, OTOH, that should not (/cannot) keep running when
    + * given an unexpected NULL pointer, should just crash. On some architectures,
    + * a NULL dereference will always reliably produce an Oops. On others, where
    + * the zero address can be mmapped, an Oops is not guaranteed. Relying on
    + * NULL dereference Oopses to happen on these architectures might lead to
    + * data corruptions (system will keep running despite a critical bug and
    + * the results will be horribly undefined). In addition, these situations
    + * can also have security implications - we have seen several privilege
    + * escalation exploits with which an attacker gained full control over the
    + * system due to NULL dereference bugs.
    + *
    + * This macro will BUG_ON if @addr is NULL on architectures where the zero
    + * addresses can be mapped. On other architectures, where the zero address
    + * can never be mapped, this macro is compiled out, so the system will just
    + * Oops when the @addr is dereferenced.
    + *
    + * As with BUG_ON, use this macro only if a NULL @addr cannot be tolerated.
    + * If proceeding with degraded functionality is an option, it's much
    + * better to just simply check for the NULL and returning instead of crashing
    + * the system.
    + */
    +#define BUG_ON_MAPPABLE_NULL(addr) do { \
    + if (arch_mmap_check(0, 1, MAP_FIXED) == 0) BUG_ON(!addr); \
    +} while(0)
    * WARN(), WARN_ON(), WARN_ON_ONCE, and so on can be used to report
    * significant issues that need prompt attention if they should ever
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to
    More majordomo info at
    Please read the FAQ at
     \ /
      Last update: 2010-11-29 10:49    [W:0.038 / U:22.984 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site