Skip to content
  • Linus Torvalds's avatar
    Fix subtle macro variable shadowing in min_not_zero() · e9092d0d
    Linus Torvalds authored
    Commit 3c8ba0d6 ("kernel.h: Retain constant expression output for
    max()/min()") rewrote our min/max macros to be very clever, but in the
    meantime resurrected a variable name shadow issue that we had had
    previously fixed in commit 589a9785 ("min/max: remove sparse
    warnings when they're nested").
    
    That commit talks about the sparse warnings that this shadowing causes,
    which we ignored as just a minor annoyance.  But it turns out that the
    sparse warning is the least of our problems.  We actually have a real
    bug due to the shadowing through the interaction with "min_not_zero()",
    which ends up doing
    
       min(__x, __y)
    
    internally, and then the new declaration of "__x" and "__y" as new
    variables in __cmp_once() results in a complete mess of an expression,
    and "min_not_zero()" doesn't work at all.
    
    For some odd reason, this only ever caused (reported) problems on s390,
    even though it is a generic issue and most of the (obviously successful)
    testing of the problematic commit had happened on other architectures.
    
    Quoting Sebastian Ott:
     "What happened is that the bio build by the partition detection code
      was attempted to be split by the block layer because the block queue
      had a max_sector setting of 0. blk_queue_max_hw_sectors uses
      min_not_zero."
    
    So re-introduce the use of __UNIQUE_ID() to make sure that the min/max
    macros do not have these kinds of clashes.
    
    [ That said, __UNIQUE_ID() itself has several issues that make it less
      than wonderful.
    
      In particular, the "uniqueness" has a fallback on the line number,
      which means that it's not actually unique in more complex cases if you
      don't build with gcc or clang (which have working unique counters that
      aren't tied to line numbers).
    
      That historical broken fallback also means that we have that pointless
      "prefix" argument that doesn't actually make much sense _except_ for
      the known-broken case. Oh well. ]
    
    Fixes: 3c8ba0d6
    
     ("kernel.h: Retain constant expression output for max()/min()")
    Reported-and-tested-by: default avatarSebastian Ott <sebott@linux.vnet.ibm.com>
    Cc: Kees Cook <keescook@chromium.org>
    Cc: Ingo Molnar <mingo@kernel.org>
    Cc: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
    Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
    e9092d0d