1. 09 Aug, 2018 1 commit
    • Al Viro's avatar
      make sure that __dentry_kill() always invalidates d_seq, unhashed or not · 4c0d7cd5
      Al Viro authored
      RCU pathwalk relies upon the assumption that anything that changes
      ->d_inode of a dentry will invalidate its ->d_seq.  That's almost
      true - the one exception is that the final dput() of already unhashed
      dentry does *not* touch ->d_seq at all.  Unhashing does, though,
      so for anything we'd found by RCU dcache lookup we are fine.
      Unfortunately, we can *start* with an unhashed dentry or jump into
      We could try and be careful in the (few) places where that could
      happen.  Or we could just make the final dput() invalidate the damn
      thing, unhashed or not.  The latter is much simpler and easier to
      backport, so let's do it that way.
      Reported-by: default avatar"Dae R. Jeong" <threeearcat@gmail.com>
      Cc: stable@vger.kernel.org
      Signed-off-by: default avatarAl Viro <viro@zeniv.linux.org.uk>
  2. 06 Aug, 2018 1 commit
    • Al Viro's avatar
      root dentries need RCU-delayed freeing · 90bad5e0
      Al Viro authored
      Since mountpoint crossing can happen without leaving lazy mode,
      root dentries do need the same protection against having their
      memory freed without RCU delay as everything else in the tree.
      It's partially hidden by RCU delay between detaching from the
      mount tree and dropping the vfsmount reference, but the starting
      point of pathwalk can be on an already detached mount, in which
      case umount-caused RCU delay has already passed by the time the
      lazy pathwalk grabs rcu_read_lock().  If the starting point
      happens to be at the root of that vfsmount *and* that vfsmount
      covers the entire filesystem, we get trouble.
      Fixes: 48a066e7 ("RCU'd vsfmounts")
      Cc: stable@vger.kernel.org
      Signed-off-by: default avatarAl Viro <viro@zeniv.linux.org.uk>
  3. 13 May, 2018 1 commit
    • Al Viro's avatar
      get rid of dead code in d_find_alias() · 61fec493
      Al Viro authored
      All "try disconnected alias if nothing else fits" logics in d_find_alias()
      got accidentally disabled by Neil a while ago; for most of the callers it
      was the right thing to do, so fixes belong in few callers that *do* want
      disconnected aliases.  This just takes the now-dead code in d_find_alias()
      Signed-off-by: default avatarAl Viro <viro@zeniv.linux.org.uk>
  4. 11 May, 2018 1 commit
    • Al Viro's avatar
      do d_instantiate/unlock_new_inode combinations safely · 1e2e547a
      Al Viro authored
      For anything NFS-exported we do _not_ want to unlock new inode
      before it has grown an alias; original set of fixes got the
      ordering right, but missed the nasty complication in case of
      lockdep being enabled - unlock_new_inode() does
      which can only be done before anyone gets a chance to touch
      ->i_mutex.  Unfortunately, flipping the order and doing
      unlock_new_inode() before d_instantiate() opens a window when
      mkdir can race with open-by-fhandle on a guessed fhandle, leading
      to multiple aliases for a directory inode and all the breakage
      that follows from that.
      	Correct solution: a new primitive (d_instantiate_new())
      combining these two in the right order - lockdep annotate, then
      d_instantiate(), then the rest of unlock_new_inode().  All
      combinations of d_instantiate() with unlock_new_inode() should
      be converted to that.
      Cc: stable@kernel.org	# 2.6.29 and later
      Tested-by: default avatarMike Marshall <hubcap@omnibond.com>
      Reviewed-by: default avatarAndreas Dilger <adilger@dilger.ca>
      Signed-off-by: default avatarAl Viro <viro@zeniv.linux.org.uk>
  5. 20 Apr, 2018 1 commit
  6. 16 Apr, 2018 4 commits
  7. 11 Apr, 2018 2 commits
    • Nikolay Borisov's avatar
      fs/dcache.c: add cond_resched() in shrink_dentry_list() · 32785c05
      Nikolay Borisov authored
      As previously reported (https://patchwork.kernel.org/patch/8642031/)
      it's possible to call shrink_dentry_list with a large number of dentries
      (> 10000).  This, in turn, could trigger the softlockup detector and
      possibly trigger a panic.  In addition to the unmount path being
      vulnerable to this scenario, at SuSE we've observed similar situation
      happening during process exit on processes that touch a lot of dentries.
      Here is an excerpt from a crash dump.  The number after the colon are
      the number of dentries on the list passed to shrink_dentry_list:
      PID 99760: 10722
      PID 107530: 215
      PID 108809: 24134
      PID 108877: 21331
      PID 141708: 16487
      So we want to kill between 15k-25k dentries without yielding.
      And one possible call stack looks like:
      4 [ffff8839ece41db0] _raw_spin_lock at ffffffff8152a5f8
      5 [ffff8839ece41db0] evict at ffffffff811c3026
      6 [ffff8839ece41dd0] __dentry_kill at ffffffff811bf258
      7 [ffff8839ece41df0] shrink_dentry_list at ffffffff811bf593
      8 [ffff8839ece41e18] shrink_dcache_parent at ffffffff811bf830
      9 [ffff8839ece41e50] proc_flush_task at ffffffff8120dd61
      10 [ffff8839ece41ec0] release_task at ffffffff81059ebd
      11 [ffff8839ece41f08] do_exit at ffffffff8105b8ce
      12 [ffff8839ece41f78] sys_exit at ffffffff8105bd53
      13 [ffff8839ece41f80] system_call_fastpath at ffffffff81532909
      While some of the callers of shrink_dentry_list do use cond_resched,
      this is not sufficient to prevent softlockups.  So just move
      cond_resched into shrink_dentry_list from its callers.
      David said: I've found hundreds of occurrences of warnings that we emit
      when need_resched stays set for a prolonged period of time with the
      stack trace that is included in the change log.
      Link: http://lkml.kernel.org/r/1521718946-31521-1-git-send-email-nborisov@suse.comSigned-off-by: default avatarNikolay Borisov <nborisov@suse.com>
      Reviewed-by: default avatarAndrew Morton <akpm@linux-foundation.org>
      Acked-by: default avatarDavid Rientjes <rientjes@google.com>
      Cc: Alexander Viro <viro@zeniv.linux.org.uk>
      Cc: Goldwyn Rodrigues <rgoldwyn@suse.de>
      Cc: Jeff Mahoney <jeffm@suse.com>
      Cc: Davidlohr Bueso <dave@stgolabs.net>
      Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
      Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
    • Roman Gushchin's avatar
      dcache: account external names as indirectly reclaimable memory · f1782c9b
      Roman Gushchin authored
      I received a report about suspicious growth of unreclaimable slabs on
      some machines.  I've found that it happens on machines with low memory
      pressure, and these unreclaimable slabs are external names attached to
      External names are allocated using generic kmalloc() function, so they
      are accounted as unreclaimable.  But they are held by dentries, which
      are reclaimable, and they will be reclaimed under the memory pressure.
      In particular, this breaks MemAvailable calculation, as it doesn't take
      unreclaimable slabs into account.  This leads to a silly situation, when
      a machine is almost idle, has no memory pressure and therefore has a big
      dentry cache.  And the resulting MemAvailable is too low to start a new
      To address the issue, the NR_INDIRECTLY_RECLAIMABLE_BYTES counter is
      used to track the amount of memory, consumed by external names.  The
      counter is increased in the dentry allocation path, if an external name
      structure is allocated; and it's decreased in the dentry freeing path.
      To reproduce the problem I've used the following Python script:
        import os
        for iter in range (0, 10000000):
                name = ("/some_long_name_%d" % iter) + "_" * 220
            except Exception:
      Without this patch:
        $ cat /proc/meminfo | grep MemAvailable
        MemAvailable:    7811688 kB
        $ python indirect.py
        $ cat /proc/meminfo | grep MemAvailable
        MemAvailable:    2753052 kB
      With the patch:
        $ cat /proc/meminfo | grep MemAvailable
        MemAvailable:    7809516 kB
        $ python indirect.py
        $ cat /proc/meminfo | grep MemAvailable
        MemAvailable:    7749144 kB
      [guro@fb.com: fix indirectly reclaimable memory accounting for CONFIG_SLOB]
        Link: http://lkml.kernel.org/r/20180312194140.19517-1-guro@fb.com
      [guro@fb.com: fix indirectly reclaimable memory accounting]
        Link: http://lkml.kernel.org/r/20180313125701.7955-1-guro@fb.com
      Link: http://lkml.kernel.org/r/20180305133743.12746-5-guro@fb.comSigned-off-by: default avatarRoman Gushchin <guro@fb.com>
      Reviewed-by: default avatarAndrew Morton <akpm@linux-foundation.org>
      Cc: Alexander Viro <viro@zeniv.linux.org.uk>
      Cc: Michal Hocko <mhocko@suse.com>
      Cc: Johannes Weiner <hannes@cmpxchg.org>
      Cc: Mel Gorman <mgorman@techsingularity.net>
      Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
      Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
  8. 29 Mar, 2018 12 commits
    • Al Viro's avatar
      d_genocide: move export to definition · cbd4a5bc
      Al Viro authored
      Signed-off-by: default avatarAl Viro <viro@zeniv.linux.org.uk>
    • Al Viro's avatar
    • Al Viro's avatar
      make non-exchanging __d_move() copy ->d_parent rather than swap them · 076515fc
      Al Viro authored
      Currently d_move(from, to) does the following:
      	* name/parent of from <- old name/parent of to, from hashed there
      	* to is unhashed
      	* name of to is preserved
      	* if from used to be detached, to gets detached
      	* if from used to be attached, parent of to <- old parent of from.
      That's both user-visibly bogus and complicates reasoning a lot.
      Much saner semantics would be
      	* name/parent of from <- name/parent of to, from hashed there.
      	* to is unhashed
      	* name/parent of to is unchanged.
      The price, of course, is that old parent of from might lose a reference.
      	* all potentially cross-directory callers of d_move() have both
      parents pinned directly; typically, dentries themselves are grabbed
      only after we have grabbed and locked both parents.  IOW, the decrement
      of old parent's refcount in case of d_move() won't reach zero.
      	* __d_move() from d_splice_alias() is done to detached alias.
      No refcount decrements in that case
      	* __d_move() from __d_unalias() *can* get the refcount to zero.
      So let's grab a reference to alias' old parent before calling __d_unalias()
      and dput() it after we'd dropped rename_lock.
      That does make d_splice_alias() potentially blocking.  However, it has
      no callers in non-sleepable contexts (and the case where we'd grown
      that dget/dput pair is _very_ rare, so performance is not an issue).
      Another thing that needs adjustment is unlocking in the end of __d_move();
      folded it in.  And cleaned the remnants of bogus ordering from the
      "lock them in the beginning" counterpart - it's never been right and
      now (well, for 7 years now) we have that thing always serialized on
      rename_lock anyway.
      Signed-off-by: default avatarAl Viro <viro@zeniv.linux.org.uk>
    • Al Viro's avatar
      split d_path() and friends into a separate file · 7a5cf791
      Al Viro authored
      Those parts of fs/dcache.c are pretty much self-contained.
      Signed-off-by: default avatarAl Viro <viro@zeniv.linux.org.uk>
    • Al Viro's avatar
      dcache.c: trim includes · 43986d63
      Al Viro authored
      Signed-off-by: default avatarAl Viro <viro@zeniv.linux.org.uk>
    • John Ogness's avatar
      fs/dcache: Avoid a try_lock loop in shrink_dentry_list() · 8f04da2a
      John Ogness authored
      shrink_dentry_list() holds dentry->d_lock and needs to acquire
      dentry->d_inode->i_lock. This cannot be done with a spin_lock()
      operation because it's the reverse of the regular lock order.
      To avoid ABBA deadlocks it is done with a trylock loop.
      Trylock loops are problematic in two scenarios:
        1) PREEMPT_RT converts spinlocks to 'sleeping' spinlocks, which are
           preemptible. As a consequence the i_lock holder can be preempted
           by a higher priority task. If that task executes the trylock loop
           it will do so forever and live lock.
        2) In virtual machines trylock loops are problematic as well. The
           VCPU on which the i_lock holder runs can be scheduled out and a
           task on a different VCPU can loop for a whole time slice. In the
           worst case this can lead to starvation. Commits 47be6184
           ("fs/dcache.c: avoid soft-lockup in dput()") and 046b961b
           ("shrink_dentry_list(): take parent's d_lock earlier") are
           addressing exactly those symptoms.
      Avoid the trylock loop by using dentry_kill(). When pruning ancestors,
      the same code applies that is used to kill a dentry in dput(). This
      also has the benefit that the locking order is now the same. First
      the inode is locked, then the parent.
      Signed-off-by: default avatarJohn Ogness <john.ogness@linutronix.de>
      Signed-off-by: default avatarAl Viro <viro@zeniv.linux.org.uk>
    • Al Viro's avatar
      get rid of trylock loop around dentry_kill() · f657a666
      Al Viro authored
      In case when trylock in there fails, deal with it directly in
      dentry_kill().  Note that in cases when we drop and retake
      ->d_lock, we need to recheck whether to retain the dentry.
      Another thing is that dropping/retaking ->d_lock might have
      ended up with negative dentry turning into positive; that,
      of course, can happen only once...
      Signed-off-by: default avatarAl Viro <viro@zeniv.linux.org.uk>
    • Al Viro's avatar
      handle move to LRU in retain_dentry() · 62d9956c
      Al Viro authored
      Signed-off-by: default avatarAl Viro <viro@zeniv.linux.org.uk>
    • Al Viro's avatar
    • Al Viro's avatar
      split the slow part of lock_parent() off · 8b987a46
      Al Viro authored
      Turn the "trylock failed" part into uninlined __lock_parent().
      Signed-off-by: default avatarAl Viro <viro@zeniv.linux.org.uk>
    • Al Viro's avatar
      now lock_parent() can't run into killed dentry · 65d8eb5a
      Al Viro authored
      all remaining callers hold either a reference or ->i_lock
      Signed-off-by: default avatarAl Viro <viro@zeniv.linux.org.uk>
    • Al Viro's avatar
      get rid of trylock loop in locking dentries on shrink list · 3b3f09f4
      Al Viro authored
      In case of trylock failure don't re-add to the list - drop the locks
      and carefully get them in the right order.  For shrink_dentry_list(),
      somebody having grabbed a reference to dentry means that we can
      kick it off-list, so if we find dentry being modified under us we
      don't need to play silly buggers with retries anyway - off the list
      it is.
      The locking logics taken out into a helper of its own; lock_parent()
      is no longer used for dentries that can be killed under us.
      [fix from Eric Biggers folded]
      Signed-off-by: default avatarAl Viro <viro@zeniv.linux.org.uk>
  9. 12 Mar, 2018 4 commits
  10. 25 Feb, 2018 2 commits
  11. 24 Feb, 2018 1 commit
    • Al Viro's avatar
      lock_parent() needs to recheck if dentry got __dentry_kill'ed under it · 3b821409
      Al Viro authored
      In case when dentry passed to lock_parent() is protected from freeing only
      by the fact that it's on a shrink list and trylock of parent fails, we
      could get hit by __dentry_kill() (and subsequent dentry_kill(parent))
      between unlocking dentry and locking presumed parent.  We need to recheck
      that dentry is alive once we lock both it and parent *and* postpone
      rcu_read_unlock() until after that point.  Otherwise we could return
      a pointer to struct dentry that already is rcu-scheduled for freeing, with
      ->d_lock held on it; caller's subsequent attempt to unlock it can end
      up with memory corruption.
      Cc: stable@vger.kernel.org # 3.12+, counting backports
      Signed-off-by: default avatarAl Viro <viro@zeniv.linux.org.uk>
  12. 01 Feb, 2018 2 commits
  13. 26 Jan, 2018 2 commits
  14. 24 Jan, 2018 2 commits
  15. 15 Jan, 2018 2 commits
    • David Windsor's avatar
      vfs: Define usercopy region in names_cache slab caches · 6a9b8820
      David Windsor authored
      VFS pathnames are stored in the names_cache slab cache, either inline
      or across an entire allocation entry (when approaching PATH_MAX). These
      are copied to/from userspace, so they must be entirely whitelisted.
      cache object allocation:
              #define __getname()    kmem_cache_alloc(names_cachep, GFP_KERNEL)
      example usage trace:
                  result = __getname();
                  kname = (char *)result->iname;
                  result->name = kname;
                  len = strncpy_from_user(kname, filename, EMBEDDED_NAME_MAX);
                  if (unlikely(len == EMBEDDED_NAME_MAX)) {
                      const size_t size = offsetof(struct filename, iname[1]);
                      kname = (char *)result;
                      result = kzalloc(size, GFP_KERNEL);
                      result->name = kname;
                      len = strncpy_from_user(kname, filename, PATH_MAX);
      In support of usercopy hardening, this patch defines the entire cache
      object in the names_cache slab cache as whitelisted, since it may entirely
      hold name strings to be copied to/from userspace.
      This patch is verbatim from Brad Spengler/PaX Team's PAX_USERCOPY
      whitelisting code in the last public patch of grsecurity/PaX based on my
      understanding of the code. Changes or omissions from the original code are
      mine and don't reflect the original grsecurity/PaX code.
      Signed-off-by: default avatarDavid Windsor <dave@nullcore.net>
      [kees: adjust commit log, add usage trace]
      Cc: Alexander Viro <viro@zeniv.linux.org.uk>
      Cc: linux-fsdevel@vger.kernel.org
      Signed-off-by: default avatarKees Cook <keescook@chromium.org>
    • David Windsor's avatar
      dcache: Define usercopy region in dentry_cache slab cache · 80344266
      David Windsor authored
      When a dentry name is short enough, it can be stored directly in the
      dentry itself (instead in a separate kmalloc allocation). These dentry
      short names, stored in struct dentry.d_iname and therefore contained in
      the dentry_cache slab cache, need to be coped to userspace.
      cache object allocation:
                  dentry = kmem_cache_alloc(dentry_cache, ...);
                  dentry->d_name.name = dentry->d_iname;
      example usage trace:
              (called via ctx.actor by dir_emit)
              filldir(..., const char *name, ...):
                  copy_to_user(..., name, namlen)
                  next = next_positive(dentry, p, 1)
                  dir_emit(..., next->d_name.name, ...)
      In support of usercopy hardening, this patch defines a region in the
      dentry_cache slab cache in which userspace copy operations are allowed.
      This region is known as the slab cache's usercopy region. Slab caches can
      now check that each dynamic copy operation involving cache-managed memory
      falls entirely within the slab's usercopy region.
      This patch is modified from Brad Spengler/PaX Team's PAX_USERCOPY
      whitelisting code in the last public patch of grsecurity/PaX based on my
      understanding of the code. Changes or omissions from the original code are
      mine and don't reflect the original grsecurity/PaX code.
      Signed-off-by: default avatarDavid Windsor <dave@nullcore.net>
      [kees: adjust hunks for kmalloc-specific things moved later]
      [kees: adjust commit log, provide usage trace]
      Cc: Alexander Viro <viro@zeniv.linux.org.uk>
      Cc: linux-fsdevel@vger.kernel.org
      Signed-off-by: default avatarKees Cook <keescook@chromium.org>
  16. 28 Dec, 2017 1 commit
    • NeilBrown's avatar
      VFS: close race between getcwd() and d_move() · 61647823
      NeilBrown authored
      d_move() will call __d_drop() and then __d_rehash()
      on the dentry being moved.  This creates a small window
      when the dentry appears to be unhashed.  Many tests
      of d_unhashed() are made under ->d_lock and so are safe
      from racing with this window, but some aren't.
      In particular, getcwd() calls d_unlinked() (which calls
      d_unhashed()) without d_lock protection, so it can race.
      This races has been seen in practice with lustre, which uses d_move() as
      part of name lookup.  See:
      It could race with a regular rename(), and result in ENOENT instead
      of either the 'before' or 'after' name.
      The race can be demonstrated with a simple program which
      has two threads, one renaming a directory back and forth
      while another calls getcwd() within that directory: it should never
      fail, but does.  See:
      We could fix this race by taking d_lock and rechecking when
      d_unhashed() reports true.  Alternately when can remove the window,
      which is the approach this patch takes.
      ___d_drop() is introduce which does *not* clear d_hash.pprev
      so the dentry still appears to be hashed.  __d_drop() calls
      ___d_drop(), then clears d_hash.pprev.
      __d_move() now uses ___d_drop() and only clears d_hash.pprev
      when not rehashing.
      Signed-off-by: default avatarNeilBrown <neilb@suse.com>
      Signed-off-by: default avatarAl Viro <viro@zeniv.linux.org.uk>
  17. 26 Dec, 2017 1 commit
    • NeilBrown's avatar
      VFS: don't keep disconnected dentries on d_anon · f1ee6162
      NeilBrown authored
      The original purpose of the per-superblock d_anon list was to
      keep disconnected dentries in the cache between consecutive
      requests to the NFS server.  Dentries can be disconnected if
      a client holds a file open and repeatedly performs IO on it,
      and if the server drops the dentry, whether due to memory
      pressure, server restart, or "echo 3 > /proc/sys/vm/drop_caches".
      This purpose was thwarted by commit 75a6f82a ("freeing unlinked
      file indefinitely delayed") which caused disconnected dentries
      to be freed as soon as their refcount reached zero.
      This means that, when a dentry being used by nfsd gets disconnected, a
      new one needs to be allocated for every request (unless requests
      overlap).  As the dentry has no name, no parent, and no children,
      there is little of value to cache.  As small memory allocations are
      typically fast (from per-cpu free lists) this likely has little cost.
      This means that the original purpose of s_anon is no longer relevant:
      there is no longer any need to keep disconnected dentries on a list so
      they appear to be hashed.
      However, s_anon now has a new use.  When you mount an NFS filesystem,
      the dentry stored in s_root is just a placebo.  The "real" root dentry
      is allocated using d_obtain_root() and so it kept on the s_anon list.
      I don't know the reason for this, but suspect it related to NFSv4
      where a mount of "server:/some/path" require NFS to look up the root
      filehandle on the server, then walk down "/some" and "/path" to get
      the filehandle to mount.
      Whatever the reason, NFS depends on the s_anon list and on
      shrink_dcache_for_umount() pruning all dentries on this list.  So we
      cannot simply remove s_anon.
      We could just leave the code unchanged, but apart from that being
      potentially confusing, the (unfair) bit-spin-lock which protects
      s_anon can become a bottle neck when lots of disconnected dentries are
      being created.
      So this patch renames s_anon to s_roots, and stops storing
      disconnected dentries on the list.  Only dentries obtained with
      d_obtain_root() are now stored on this list.  There are many fewer of
      these (only NFS and NILFS2 use the call, and only during filesystem
      mount) so contention on the bit-lock will not be a problem.
      Possibly an alternate solution should be found for NFS and NILFS2, but
      that would require understanding their needs first.
      Signed-off-by: default avatarNeilBrown <neilb@suse.com>
      Signed-off-by: default avatarAl Viro <viro@zeniv.linux.org.uk>