• Huang Ying's avatar
    mm, swap: remove unnecessary smp_rmb() in swap_type_to_swap_info() · a4b45114
    Huang Ying authored
    Before commit c10d38cc ("mm, swap: bounds check swap_info array
    accesses to avoid NULL derefs"), the typical code to reference the
    swap_info[] is as follows,
    
      type = swp_type(swp_entry);
      if (type >= nr_swapfiles)
              /* handle invalid swp_entry */;
      p = swap_info[type];
      /* access fields of *p.  OOPS! p may be NULL! */
    
    Because the ordering isn't guaranteed, it's possible that swap_info[type]
    is read before "nr_swapfiles".  And that may result in NULL pointer
    dereference.
    
    So after commit c10d38cc, the code becomes,
    
      struct swap_info_struct *swap_type_to_swap_info(int type)
      {
    	  if (type >= READ_ONCE(nr_swapfiles))
    		  return NULL;
    	  smp_rmb();
    	  return READ_ONCE(swap_info[type]);
      }
    
      /* users */
      type = swp_type(swp_entry);
      p = swap_type_to_swap_info(type);
      if (!p)
    	  /* handle invalid swp_entry */;
      /* dereference p */
    
    Where the value of swap_info[type] (that is, "p") is checked to be
    non-zero before being dereferenced.  So, the NULL deferencing becomes
    impossible even if "nr_swapfiles" is read after swap_info[type].
    Therefore, the "smp_rmb()" becomes unnecessary.
    
    And, we don't even need to read "nr_swapfiles" here.  Because the non-zero
    checking for "p" is sufficient.  We just need to make sure we will not
    access out of the boundary of the array.  With the change, nr_swapfiles
    will only be accessed with swap_lock held, except in
    swapcache_free_entries().  Where the absolute correctness of the value
    isn't needed, as described in the comments.
    
    We still need to guarantee swap_info[type] is read before being
    dereferenced.  That can be satisfied via the data dependency ordering
    enforced by READ_ONCE(swap_info[type]).  This needs to be paired with
    proper write barriers.  So smp_store_release() is used in
    alloc_swap_info() to guarantee the fields of *swap_info[type] is
    initialized before swap_info[type] itself being written.  Note that the
    fields of *swap_info[type] is initialized to be 0 via kvzalloc() firstly.
    The assignment and deferencing of swap_info[type] is like
    rcu_assign_pointer() and rcu_dereference().
    
    Link: https://lkml.kernel.org/r/20210520073301.1676294-1-ying.huang@intel.comSigned-off-by: default avatar"Huang, Ying" <ying.huang@intel.com>
    Cc: Daniel Jordan <daniel.m.jordan@oracle.com>
    Cc: Dan Carpenter <dan.carpenter@oracle.com>
    Cc: Andrea Parri <andrea.parri@amarulasolutions.com>
    Cc: Peter Zijlstra (Intel) <peterz@infradead.org>
    Cc: Andi Kleen <ak@linux.intel.com>
    Cc: Dave Hansen <dave.hansen@linux.intel.com>
    Cc: Omar Sandoval <osandov@fb.com>
    Cc: Paul McKenney <paulmck@kernel.org>
    Cc: Tejun Heo <tj@kernel.org>
    Cc: Will Deacon <will.deacon@arm.com>
    Cc: Miaohe Lin <linmiaohe@huawei.com>
    Cc: Hugh Dickins <hughd@google.com>
    Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
    Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
    a4b45114
swapfile.c 96.2 KB