afs: Fix use-after-free due to get/remove race in volume tree
[ Upstream commit 9a6b294ab496650e9f270123730df37030911b55 ]
When an afs_volume struct is put, its refcount is reduced to 0 before
the cell->volume_lock is taken and the volume removed from the
cell->volumes tree.
Unfortunately, this means that the lookup code can race and see a volume
with a zero ref in the tree, resulting in a use-after-free:
refcount_t: addition on 0; use-after-free.
WARNING: CPU: 3 PID: 130782 at lib/refcount.c:25 refcount_warn_saturate+0x7a/0xda
...
RIP: 0010:refcount_warn_saturate+0x7a/0xda
...
Call Trace:
afs_get_volume+0x3d/0x55
afs_create_volume+0x126/0x1de
afs_validate_fc+0xfe/0x130
afs_get_tree+0x20/0x2e5
vfs_get_tree+0x1d/0xc9
do_new_mount+0x13b/0x22e
do_mount+0x5d/0x8a
__do_sys_mount+0x100/0x12a
do_syscall_64+0x3a/0x94
entry_SYSCALL_64_after_hwframe+0x62/0x6a
Fix this by:
(1) When putting, use a flag to indicate if the volume has been removed
from the tree and skip the rb_erase if it has.
(2) When looking up, use a conditional ref increment and if it fails
because the refcount is 0, replace the node in the tree and set the
removal flag.
Fixes: 20325960f8
("afs: Reorganise volume and server trees to be rooted on the cell")
Signed-off-by: David Howells <dhowells@redhat.com>
Reviewed-by: Jeffrey Altman <jaltman@auristor.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: linux-afs@lists.infradead.org
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
This commit is contained in:
parent
17605162e2
commit
9b4c95a63e
@ -585,6 +585,7 @@ struct afs_volume {
|
||||
#define AFS_VOLUME_OFFLINE 4 /* - T if volume offline notice given */
|
||||
#define AFS_VOLUME_BUSY 5 /* - T if volume busy notice given */
|
||||
#define AFS_VOLUME_MAYBE_NO_IBULK 6 /* - T if some servers don't have InlineBulkStatus */
|
||||
#define AFS_VOLUME_RM_TREE 7 /* - Set if volume removed from cell->volumes */
|
||||
#ifdef CONFIG_AFS_FSCACHE
|
||||
struct fscache_volume *cache; /* Caching cookie */
|
||||
#endif
|
||||
@ -1517,6 +1518,7 @@ extern struct afs_vlserver_list *afs_extract_vlserver_list(struct afs_cell *,
|
||||
extern struct afs_volume *afs_create_volume(struct afs_fs_context *);
|
||||
extern int afs_activate_volume(struct afs_volume *);
|
||||
extern void afs_deactivate_volume(struct afs_volume *);
|
||||
bool afs_try_get_volume(struct afs_volume *volume, enum afs_volume_trace reason);
|
||||
extern struct afs_volume *afs_get_volume(struct afs_volume *, enum afs_volume_trace);
|
||||
extern void afs_put_volume(struct afs_net *, struct afs_volume *, enum afs_volume_trace);
|
||||
extern int afs_check_volume_status(struct afs_volume *, struct afs_operation *);
|
||||
|
@ -32,8 +32,13 @@ static struct afs_volume *afs_insert_volume_into_cell(struct afs_cell *cell,
|
||||
} else if (p->vid > volume->vid) {
|
||||
pp = &(*pp)->rb_right;
|
||||
} else {
|
||||
volume = afs_get_volume(p, afs_volume_trace_get_cell_insert);
|
||||
goto found;
|
||||
if (afs_try_get_volume(p, afs_volume_trace_get_cell_insert)) {
|
||||
volume = p;
|
||||
goto found;
|
||||
}
|
||||
|
||||
set_bit(AFS_VOLUME_RM_TREE, &volume->flags);
|
||||
rb_replace_node_rcu(&p->cell_node, &volume->cell_node, &cell->volumes);
|
||||
}
|
||||
}
|
||||
|
||||
@ -56,7 +61,8 @@ static void afs_remove_volume_from_cell(struct afs_volume *volume)
|
||||
afs_volume_trace_remove);
|
||||
write_seqlock(&cell->volume_lock);
|
||||
hlist_del_rcu(&volume->proc_link);
|
||||
rb_erase(&volume->cell_node, &cell->volumes);
|
||||
if (!test_and_set_bit(AFS_VOLUME_RM_TREE, &volume->flags))
|
||||
rb_erase(&volume->cell_node, &cell->volumes);
|
||||
write_sequnlock(&cell->volume_lock);
|
||||
}
|
||||
}
|
||||
@ -235,6 +241,20 @@ static void afs_destroy_volume(struct afs_net *net, struct afs_volume *volume)
|
||||
_leave(" [destroyed]");
|
||||
}
|
||||
|
||||
/*
|
||||
* Try to get a reference on a volume record.
|
||||
*/
|
||||
bool afs_try_get_volume(struct afs_volume *volume, enum afs_volume_trace reason)
|
||||
{
|
||||
int r;
|
||||
|
||||
if (__refcount_inc_not_zero(&volume->ref, &r)) {
|
||||
trace_afs_volume(volume->vid, r + 1, reason);
|
||||
return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
/*
|
||||
* Get a reference on a volume record.
|
||||
*/
|
||||
|
Loading…
Reference in New Issue
Block a user