linux-kernel - [RFC][PATCH 0/5] Improve static call NULL handling

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-next>] [day] [month] [year] [list]

Date:   Fri, 10 Mar 2023 12:31:12 -0800
From:   Josh Poimboeuf <jpoimboe@...nel.org>
To:     x86@...nel.org
Cc:     linux-kernel@...r.kernel.org,
        Peter Zijlstra <peterz@...radead.org>,
        Mark Rutland <mark.rutland@....com>,
        Jason Baron <jbaron@...mai.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        Ard Biesheuvel <ardb@...nel.org>,
        Christophe Leroy <christophe.leroy@...roup.eu>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Sean Christopherson <seanjc@...gle.com>
Subject: [RFC][PATCH 0/5] Improve static call NULL handling

Static calling a NULL pointer is a NOP, unless you're one of those poor
souls running on an arch (or backported x86 monstrosity) with
CONFIG_HAVE_STATIC_CALL=n, then it's a panic.

The "fix" for this undefined behavior is to tell the user to just use
static_call_cond() instead, if they want consistent NOP behavior.  But
forgetting to do that is likely to cause subtle bugs.  It actually
already did (during RHEL development).

There are two ways to make it consistent:

  a) Make static_call(NULL) a NOP for all configs; or

  b) Make static_call(NULL) a panic for all configs.

Do (a) because it's consistent with the existing HAVE_STATIC_CALL
behavior.  Also it seems simpler to implement and use, and based on
looking at the existing use cases, it's common to want the "do nothing
and return 0" behavior by default.

Then take it a step further and get rid of the distinction between
STATIC_CALL_NULL and STATIC_CALL_RET0.

The end result is less confusing semantics and simpler code all around.

EPILOGUE
--------

If any users wanted panic-on-NULL by default instead of NOP-on-NULL,
that could be added on top of this.  They could just initialize the
static call with a __static_call_bug() helper.

  void __static_call_bug(void)
  {
  	BUG();
  }
  ..
  DEFINE_STATIC_CALL(foo, (func_type)__static_call_bug);

We could take that even further:

  DEFINE_STATIC_CALL_NOP(foo, func_type);
  DEFINE_STATIC_CALL_BUG(bar, func_type);
  ...
  #define STATIC_CALL_NOP (func_type)__static_call_nop
  #define STATIC_CALL_BUG (func_type)__static_call_bug
  ...
  static_call_update(foo, STATIC_CALL_NOP); // do nothing and return 0
  static_call_update(foo, STATIC_CALL_BUG); // panic
  static_call_update(foo, NULL);	    // ???

The default behavior for NULL could be a key-specific policy, stored as
a flag in the static_call_key struct.

The key-specific policy would be easier to deal with than the
call-site-specific policy we have today with static_call_cond().

Josh Poimboeuf (5):
  static_call: Make NULL static calls consistent
  static_call: Make NULL static calls return 0
  static_call: Remove static_call_cond() and its usages
  static_call: Remove DEFINE_STATIC_CALL_RET0() and its uses
  x86/kvm: Simplify static call handling

 arch/powerpc/include/asm/static_call.h    |   1 -
 arch/powerpc/kernel/irq.c                 |   2 +-
 arch/x86/events/amd/core.c                |   2 +-
 arch/x86/events/core.c                    |  26 ++---
 arch/x86/include/asm/kvm-x86-ops.h        |  86 +++++++-------
 arch/x86/include/asm/kvm-x86-pmu-ops.h    |  17 +--
 arch/x86/include/asm/kvm_host.h           |   6 +-
 arch/x86/include/asm/static_call.h        |   8 --
 arch/x86/kvm/irq.c                        |   2 +-
 arch/x86/kvm/lapic.c                      |  22 ++--
 arch/x86/kvm/pmu.c                        |  11 +-
 arch/x86/kvm/x86.c                        |  36 +++---
 include/linux/static_call.h               | 131 +++++-----------------
 kernel/events/core.c                      |   8 +-
 kernel/sched/core.c                       |  10 +-
 security/keys/trusted-keys/trusted_core.c |   2 +-
 16 files changed, 126 insertions(+), 244 deletions(-)

-- 
2.39.2