mm, swap: bounds check swap_info array accesses to avoid NULL derefs
Dan Carpenter reports a potential NULL dereference in
get_swap_page_of_type:
Smatch complains that the NULL checks on "si" aren't consistent. This
seems like a real bug because we have not ensured that the type is
valid and so "si" can be NULL.
Add the missing check for NULL, taking care to use a read barrier to
ensure CPU1 observes CPU0's updates in the correct order:
CPU0 CPU1
alloc_swap_info() if (type >= nr_swapfiles)
swap_info[type] = p /* handle invalid entry */
smp_wmb() smp_rmb()
++nr_swapfiles p = swap_info[type]
Without smp_rmb, CPU1 might observe CPU0's write to nr_swapfiles before
CPU0's write to swap_info[type] and read NULL from swap_info[type].
Ying Huang noticed other places in swapfile.c don't order these reads
properly. Introduce swap_type_to_swap_info to encourage correct usage.
Use READ_ONCE and WRITE_ONCE to follow the Linux Kernel Memory Model
(see tools/memory-model/Documentation/explanation.txt).
This ordering need not be enforced in places where swap_lock is held
(e.g. si_swapinfo) because swap_lock serializes updates to nr_swapfiles
and the swap_info array.
Link: http://lkml.kernel.org/r/20190131024410.29859-1-daniel.m.jordan@oracle.com
Fixes: ec8acf20af
("swap: add per-partition lock for swapfile")
Signed-off-by: Daniel Jordan <daniel.m.jordan@oracle.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Suggested-by: "Huang, Ying" <ying.huang@intel.com>
Reviewed-by: Andrea Parri <andrea.parri@amarulasolutions.com>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Alan Stern <stern@rowland.harvard.edu>
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@linux.vnet.ibm.com>
Cc: Shaohua Li <shli@kernel.org>
Cc: Stephen Rothwell <sfr@canb.auug.org.au>
Cc: Tejun Heo <tj@kernel.org>
Cc: Will Deacon <will.deacon@arm.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
This commit is contained in:
parent
060f005f07
commit
c10d38cc8d
1 changed files with 29 additions and 22 deletions
|
@ -98,6 +98,15 @@ static atomic_t proc_poll_event = ATOMIC_INIT(0);
|
||||||
|
|
||||||
atomic_t nr_rotate_swap = ATOMIC_INIT(0);
|
atomic_t nr_rotate_swap = ATOMIC_INIT(0);
|
||||||
|
|
||||||
|
static struct swap_info_struct *swap_type_to_swap_info(int type)
|
||||||
|
{
|
||||||
|
if (type >= READ_ONCE(nr_swapfiles))
|
||||||
|
return NULL;
|
||||||
|
|
||||||
|
smp_rmb(); /* Pairs with smp_wmb in alloc_swap_info. */
|
||||||
|
return READ_ONCE(swap_info[type]);
|
||||||
|
}
|
||||||
|
|
||||||
static inline unsigned char swap_count(unsigned char ent)
|
static inline unsigned char swap_count(unsigned char ent)
|
||||||
{
|
{
|
||||||
return ent & ~SWAP_HAS_CACHE; /* may include COUNT_CONTINUED flag */
|
return ent & ~SWAP_HAS_CACHE; /* may include COUNT_CONTINUED flag */
|
||||||
|
@ -1044,12 +1053,14 @@ noswap:
|
||||||
/* The only caller of this function is now suspend routine */
|
/* The only caller of this function is now suspend routine */
|
||||||
swp_entry_t get_swap_page_of_type(int type)
|
swp_entry_t get_swap_page_of_type(int type)
|
||||||
{
|
{
|
||||||
struct swap_info_struct *si;
|
struct swap_info_struct *si = swap_type_to_swap_info(type);
|
||||||
pgoff_t offset;
|
pgoff_t offset;
|
||||||
|
|
||||||
si = swap_info[type];
|
if (!si)
|
||||||
|
goto fail;
|
||||||
|
|
||||||
spin_lock(&si->lock);
|
spin_lock(&si->lock);
|
||||||
if (si && (si->flags & SWP_WRITEOK)) {
|
if (si->flags & SWP_WRITEOK) {
|
||||||
atomic_long_dec(&nr_swap_pages);
|
atomic_long_dec(&nr_swap_pages);
|
||||||
/* This is called for allocating swap entry, not cache */
|
/* This is called for allocating swap entry, not cache */
|
||||||
offset = scan_swap_map(si, 1);
|
offset = scan_swap_map(si, 1);
|
||||||
|
@ -1060,6 +1071,7 @@ swp_entry_t get_swap_page_of_type(int type)
|
||||||
atomic_long_inc(&nr_swap_pages);
|
atomic_long_inc(&nr_swap_pages);
|
||||||
}
|
}
|
||||||
spin_unlock(&si->lock);
|
spin_unlock(&si->lock);
|
||||||
|
fail:
|
||||||
return (swp_entry_t) {0};
|
return (swp_entry_t) {0};
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -1071,9 +1083,9 @@ static struct swap_info_struct *__swap_info_get(swp_entry_t entry)
|
||||||
if (!entry.val)
|
if (!entry.val)
|
||||||
goto out;
|
goto out;
|
||||||
type = swp_type(entry);
|
type = swp_type(entry);
|
||||||
if (type >= nr_swapfiles)
|
p = swap_type_to_swap_info(type);
|
||||||
|
if (!p)
|
||||||
goto bad_nofile;
|
goto bad_nofile;
|
||||||
p = swap_info[type];
|
|
||||||
if (!(p->flags & SWP_USED))
|
if (!(p->flags & SWP_USED))
|
||||||
goto bad_device;
|
goto bad_device;
|
||||||
offset = swp_offset(entry);
|
offset = swp_offset(entry);
|
||||||
|
@ -1697,10 +1709,9 @@ int swap_type_of(dev_t device, sector_t offset, struct block_device **bdev_p)
|
||||||
sector_t swapdev_block(int type, pgoff_t offset)
|
sector_t swapdev_block(int type, pgoff_t offset)
|
||||||
{
|
{
|
||||||
struct block_device *bdev;
|
struct block_device *bdev;
|
||||||
|
struct swap_info_struct *si = swap_type_to_swap_info(type);
|
||||||
|
|
||||||
if ((unsigned int)type >= nr_swapfiles)
|
if (!si || !(si->flags & SWP_WRITEOK))
|
||||||
return 0;
|
|
||||||
if (!(swap_info[type]->flags & SWP_WRITEOK))
|
|
||||||
return 0;
|
return 0;
|
||||||
return map_swap_entry(swp_entry(type, offset), &bdev);
|
return map_swap_entry(swp_entry(type, offset), &bdev);
|
||||||
}
|
}
|
||||||
|
@ -2151,7 +2162,7 @@ static sector_t map_swap_entry(swp_entry_t entry, struct block_device **bdev)
|
||||||
struct swap_extent *se;
|
struct swap_extent *se;
|
||||||
pgoff_t offset;
|
pgoff_t offset;
|
||||||
|
|
||||||
sis = swap_info[swp_type(entry)];
|
sis = swp_swap_info(entry);
|
||||||
*bdev = sis->bdev;
|
*bdev = sis->bdev;
|
||||||
|
|
||||||
offset = swp_offset(entry);
|
offset = swp_offset(entry);
|
||||||
|
@ -2593,9 +2604,7 @@ static void *swap_start(struct seq_file *swap, loff_t *pos)
|
||||||
if (!l)
|
if (!l)
|
||||||
return SEQ_START_TOKEN;
|
return SEQ_START_TOKEN;
|
||||||
|
|
||||||
for (type = 0; type < nr_swapfiles; type++) {
|
for (type = 0; (si = swap_type_to_swap_info(type)); type++) {
|
||||||
smp_rmb(); /* read nr_swapfiles before swap_info[type] */
|
|
||||||
si = swap_info[type];
|
|
||||||
if (!(si->flags & SWP_USED) || !si->swap_map)
|
if (!(si->flags & SWP_USED) || !si->swap_map)
|
||||||
continue;
|
continue;
|
||||||
if (!--l)
|
if (!--l)
|
||||||
|
@ -2615,9 +2624,7 @@ static void *swap_next(struct seq_file *swap, void *v, loff_t *pos)
|
||||||
else
|
else
|
||||||
type = si->type + 1;
|
type = si->type + 1;
|
||||||
|
|
||||||
for (; type < nr_swapfiles; type++) {
|
for (; (si = swap_type_to_swap_info(type)); type++) {
|
||||||
smp_rmb(); /* read nr_swapfiles before swap_info[type] */
|
|
||||||
si = swap_info[type];
|
|
||||||
if (!(si->flags & SWP_USED) || !si->swap_map)
|
if (!(si->flags & SWP_USED) || !si->swap_map)
|
||||||
continue;
|
continue;
|
||||||
++*pos;
|
++*pos;
|
||||||
|
@ -2724,14 +2731,14 @@ static struct swap_info_struct *alloc_swap_info(void)
|
||||||
}
|
}
|
||||||
if (type >= nr_swapfiles) {
|
if (type >= nr_swapfiles) {
|
||||||
p->type = type;
|
p->type = type;
|
||||||
swap_info[type] = p;
|
WRITE_ONCE(swap_info[type], p);
|
||||||
/*
|
/*
|
||||||
* Write swap_info[type] before nr_swapfiles, in case a
|
* Write swap_info[type] before nr_swapfiles, in case a
|
||||||
* racing procfs swap_start() or swap_next() is reading them.
|
* racing procfs swap_start() or swap_next() is reading them.
|
||||||
* (We never shrink nr_swapfiles, we never free this entry.)
|
* (We never shrink nr_swapfiles, we never free this entry.)
|
||||||
*/
|
*/
|
||||||
smp_wmb();
|
smp_wmb();
|
||||||
nr_swapfiles++;
|
WRITE_ONCE(nr_swapfiles, nr_swapfiles + 1);
|
||||||
} else {
|
} else {
|
||||||
kvfree(p);
|
kvfree(p);
|
||||||
p = swap_info[type];
|
p = swap_info[type];
|
||||||
|
@ -3251,7 +3258,7 @@ static int __swap_duplicate(swp_entry_t entry, unsigned char usage)
|
||||||
{
|
{
|
||||||
struct swap_info_struct *p;
|
struct swap_info_struct *p;
|
||||||
struct swap_cluster_info *ci;
|
struct swap_cluster_info *ci;
|
||||||
unsigned long offset, type;
|
unsigned long offset;
|
||||||
unsigned char count;
|
unsigned char count;
|
||||||
unsigned char has_cache;
|
unsigned char has_cache;
|
||||||
int err = -EINVAL;
|
int err = -EINVAL;
|
||||||
|
@ -3259,10 +3266,10 @@ static int __swap_duplicate(swp_entry_t entry, unsigned char usage)
|
||||||
if (non_swap_entry(entry))
|
if (non_swap_entry(entry))
|
||||||
goto out;
|
goto out;
|
||||||
|
|
||||||
type = swp_type(entry);
|
p = swp_swap_info(entry);
|
||||||
if (type >= nr_swapfiles)
|
if (!p)
|
||||||
goto bad_file;
|
goto bad_file;
|
||||||
p = swap_info[type];
|
|
||||||
offset = swp_offset(entry);
|
offset = swp_offset(entry);
|
||||||
if (unlikely(offset >= p->max))
|
if (unlikely(offset >= p->max))
|
||||||
goto out;
|
goto out;
|
||||||
|
@ -3359,7 +3366,7 @@ int swapcache_prepare(swp_entry_t entry)
|
||||||
|
|
||||||
struct swap_info_struct *swp_swap_info(swp_entry_t entry)
|
struct swap_info_struct *swp_swap_info(swp_entry_t entry)
|
||||||
{
|
{
|
||||||
return swap_info[swp_type(entry)];
|
return swap_type_to_swap_info(swp_type(entry));
|
||||||
}
|
}
|
||||||
|
|
||||||
struct swap_info_struct *page_swap_info(struct page *page)
|
struct swap_info_struct *page_swap_info(struct page *page)
|
||||||
|
|
Loading…
Add table
Reference in a new issue