• Will Deacon's avatar
    samples/hw_breakpoint: drop HW_BREAKPOINT_R when reporting writes · 4800314e
    Will Deacon authored
    Patch series "Unexport kallsyms_lookup_name() and kallsyms_on_each_symbol()".
    
    Despite having just a single modular in-tree user that I could spot,
    kallsyms_lookup_name() is exported to modules and provides a mechanism
    for out-of-tree modules to access and invoke arbitrary, non-exported
    kernel symbols when kallsyms is enabled.
    
    This patch series fixes up that one user and unexports the symbol along
    with kallsyms_on_each_symbol(), since that could also be abused in a
    similar manner.
    
    I would like to avoid out-of-tree modules being easily able to call
    functions that are not exported.  kallsyms_lookup_name() makes this
    trivial to the point that there is very little incentive to rework these
    modules to either use upstream interfaces correctly or propose
    functionality which may be otherwise missing upstream.  Both of these
    latter solutions would be pre-requisites to upstreaming these modules, and
    the current state of things actively discourages that approach.
    
    The background here is that we are aiming for Android devices to be able
    to use a generic binary kernel image closely following upstream, with any
    vendor extensions coming in as kernel modules.  In this case, we (Google)
    end up maintaining the binary module ABI within the scope of a single LTS
    kernel.  Monitoring and managing the ABI surface is not feasible if it
    effectively includes all data and functions via kallsyms_lookup_name().
    Of course, we could just carry this patch in the Android kernel tree, but
    we're aiming to carry as little as possible (ideally nothing) and I think
    it's a sensible change in its own right.  I'm surprised you object to it,
    in all honesty.
    
    Now, you could turn around and say "that's not upstream's problem", but it
    still seems highly undesirable to me to have an upstream bypass for
    exported symbols that isn't even used by upstream modules.  It's ripe for
    abuse and encourages people to work outside of the upstream tree.  The
    usual rule is that we don't export symbols without a user in the tree and
    that seems especially relevant in this case.
    
    Joe Lawrence said:
    
    : FWIW, kallsyms was historically used by the out-of-tree kpatch support
    : module to resolve external symbols as well as call set_memory_r{w,o}()
    : API.  All of that support code has been merged upstream, so modern kpatch
    : modules* no longer leverage kallsyms by default.
    :
    : That said, there are still some users who still use the deprecated support
    : module with newer kernels, but that is not officially supported by the
    : project.
    
    This patch (of 3):
    
    Given the name of a kernel symbol, the 'data_breakpoint' test claims to
    "report any write operations on the kernel symbol".  However, it creates
    the breakpoint using both HW_BREAKPOINT_W and HW_BREAKPOINT_R, which menas
    it also fires for read access.
    
    Drop HW_BREAKPOINT_R from the breakpoint attributes.
    Signed-off-by: default avatarWill Deacon <will@kernel.org>
    Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
    Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
    Reviewed-by: default avatarMasami Hiramatsu <mhiramat@kernel.org>
    Reviewed-by: default avatarQuentin Perret <qperret@google.com>
    Cc: K.Prasad <prasad@linux.vnet.ibm.com>
    Cc: Thomas Gleixner <tglx@linutronix.de>
    Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    Cc: Frederic Weisbecker <frederic@kernel.org>
    Cc: Alexei Starovoitov <ast@kernel.org>
    Cc: Miroslav Benes <mbenes@suse.cz>
    Cc: Petr Mladek <pmladek@suse.com>
    Cc: Joe Lawrence <joe.lawrence@redhat.com>
    Link: http://lkml.kernel.org/r/20200221114404.14641-2-will@kernel.orgSigned-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
    4800314e
data_breakpoint.c 2.19 KB