Skip to content

Commit 457f730

Browse files
brettcreeleyawilliam
authored andcommitted
vfio/pds: Make sure migration file isn't accessed after reset
It's possible the migration file is accessed after reset when it has been cleaned up, especially when it's initiated by the device. This is because the driver doesn't rip out the filep when cleaning up it only frees the related page structures and sets its local struct pds_vfio_lm_file pointer to NULL. This can cause a NULL pointer dereference, which is shown in the example below during a restore after a device initiated reset: BUG: kernel NULL pointer dereference, address: 000000000000000c PF: supervisor read access in kernel mode PF: error_code(0x0000) - not-present page PGD 0 P4D 0 Oops: 0000 [#1] PREEMPT SMP NOPTI RIP: 0010:pds_vfio_get_file_page+0x5d/0xf0 [pds_vfio_pci] [...] Call Trace: <TASK> pds_vfio_restore_write+0xf6/0x160 [pds_vfio_pci] vfs_write+0xc9/0x3f0 ? __fget_light+0xc9/0x110 ksys_write+0xb5/0xf0 __x64_sys_write+0x1a/0x20 do_syscall_64+0x38/0x90 entry_SYSCALL_64_after_hwframe+0x63/0xcd [...] Add a disabled flag to the driver's struct pds_vfio_lm_file that gets set during cleanup. Then make sure to check the flag when the migration file is accessed via its file_operations. By default this flag will be false as the memory for struct pds_vfio_lm_file is kzalloc'd, which means the struct pds_vfio_lm_file is enabled and accessible. Also, since the file_operations and driver's migration file cleanup happen under the protection of the same pds_vfio_lm_file.lock, using this flag is thread safe. Fixes: 8512ed2 ("vfio/pds: Always clear the save/restore FDs on reset") Reviewed-by: Shannon Nelson <shannon.nelson@amd.com> Signed-off-by: Brett Creeley <brett.creeley@amd.com> Link: https://lore.kernel.org/r/20240308182149.22036-2-brett.creeley@amd.com Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
1 parent 9b27b11 commit 457f730

File tree

2 files changed

+14
-0
lines changed

2 files changed

+14
-0
lines changed

drivers/vfio/pci/pds/lm.c

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,8 +92,10 @@ static void pds_vfio_put_lm_file(struct pds_vfio_lm_file *lm_file)
9292
{
9393
mutex_lock(&lm_file->lock);
9494

95+
lm_file->disabled = true;
9596
lm_file->size = 0;
9697
lm_file->alloc_size = 0;
98+
lm_file->filep->f_pos = 0;
9799

98100
/* Free scatter list of file pages */
99101
sg_free_table(&lm_file->sg_table);
@@ -183,6 +185,12 @@ static ssize_t pds_vfio_save_read(struct file *filp, char __user *buf,
183185
pos = &filp->f_pos;
184186

185187
mutex_lock(&lm_file->lock);
188+
189+
if (lm_file->disabled) {
190+
done = -ENODEV;
191+
goto out_unlock;
192+
}
193+
186194
if (*pos > lm_file->size) {
187195
done = -EINVAL;
188196
goto out_unlock;
@@ -283,6 +291,11 @@ static ssize_t pds_vfio_restore_write(struct file *filp, const char __user *buf,
283291

284292
mutex_lock(&lm_file->lock);
285293

294+
if (lm_file->disabled) {
295+
done = -ENODEV;
296+
goto out_unlock;
297+
}
298+
286299
while (len) {
287300
size_t page_offset;
288301
struct page *page;

drivers/vfio/pci/pds/lm.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ struct pds_vfio_lm_file {
2727
struct scatterlist *last_offset_sg; /* Iterator */
2828
unsigned int sg_last_entry;
2929
unsigned long last_offset;
30+
bool disabled;
3031
};
3132

3233
struct pds_vfio_pci_device;

0 commit comments

Comments
 (0)