Skip to content

Commit

Permalink
mm: cachestat: fix two shmem bugs
Browse files Browse the repository at this point in the history
commit d5d39c7 upstream.

When cachestat on shmem races with swapping and invalidation, there
are two possible bugs:

1) A swapin error can have resulted in a poisoned swap entry in the
   shmem inode's xarray. Calling get_shadow_from_swap_cache() on it
   will result in an out-of-bounds access to swapper_spaces[].

   Validate the entry with non_swap_entry() before going further.

2) When we find a valid swap entry in the shmem's inode, the shadow
   entry in the swapcache might not exist yet: swap IO is still in
   progress and we're before __remove_mapping; swapin, invalidation,
   or swapoff have removed the shadow from swapcache after we saw the
   shmem swap entry.

   This will send a NULL to workingset_test_recent(). The latter
   purely operates on pointer bits, so it won't crash - node 0, memcg
   ID 0, eviction timestamp 0, etc. are all valid inputs - but it's a
   bogus test. In theory that could result in a false "recently
   evicted" count.

   Such a false positive wouldn't be the end of the world. But for
   code clarity and (future) robustness, be explicit about this case.

   Bail on get_shadow_from_swap_cache() returning NULL.

Link: https://lkml.kernel.org/r/20240315095556.GC581298@cmpxchg.org
Fixes: cf264e1 ("cachestat: implement cachestat syscall")
Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
Reported-by: Chengming Zhou <chengming.zhou@linux.dev>	[Bug #1]
Reported-by: Jann Horn <jannh@google.com>		[Bug #2]
Reviewed-by: Chengming Zhou <chengming.zhou@linux.dev>
Reviewed-by: Nhat Pham <nphamcs@gmail.com>
Cc: <stable@vger.kernel.org>				[v6.5+]
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  • Loading branch information
hnaz authored and gregkh committed Apr 3, 2024
1 parent a079f72 commit 24a0e73
Showing 1 changed file with 16 additions and 0 deletions.
16 changes: 16 additions & 0 deletions mm/filemap.c
Expand Up @@ -4153,7 +4153,23 @@ static void filemap_cachestat(struct address_space *mapping,
/* shmem file - in swap cache */
swp_entry_t swp = radix_to_swp_entry(folio);

/* swapin error results in poisoned entry */
if (non_swap_entry(swp))
goto resched;

/*
* Getting a swap entry from the shmem
* inode means we beat
* shmem_unuse(). rcu_read_lock()
* ensures swapoff waits for us before
* freeing the swapper space. However,
* we can race with swapping and
* invalidation, so there might not be
* a shadow in the swapcache (yet).
*/
shadow = get_shadow_from_swap_cache(swp);
if (!shadow)
goto resched;
}
#endif
if (workingset_test_recent(shadow, true, &workingset))
Expand Down

0 comments on commit 24a0e73

Please sign in to comment.