linux-kernel - Re: [PATCH -next] vfio/pci: fix a null-ptr-deref in vfio_config

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Tue, 26 May 2020 10:21:27 -0600
From:   Alex Williamson <alex.williamson@...hat.com>
To:     Qian Cai <cai@....pw>
Cc:     cohuck@...hat.com, kvm@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH -next] vfio/pci: fix a null-ptr-deref in
 vfio_config_free()

On Thu, 21 May 2020 21:18:29 -0400
Qian Cai <cai@....pw> wrote:

> It is possible vfio_config_init() does not call vfio_cap_len(), and then
> vdev->msi_perm == NULL. Later, in vfio_config_free(), it could trigger a
> null-ptr-deref.
> 
>  BUG: kernel NULL pointer dereference, address: 0000000000000000
>  RIP: 0010:vfio_config_free+0x7a/0xe0 [vfio_pci]
>  vfio_config_free+0x7a/0xe0:
>  free_perm_bits at drivers/vfio/pci/vfio_pci_config.c:340
>  (inlined by) vfio_config_free at drivers/vfio/pci/vfio_pci_config.c:1760
>  Call Trace:
>   vfio_pci_release+0x3a4/0x9e0 [vfio_pci]
>   vfio_device_fops_release+0x50/0x80 [vfio]
>   __fput+0x200/0x460
>   ____fput+0xe/0x10
>   task_work_run+0x127/0x1b0
>   do_exit+0x782/0x10d0
>   do_group_exit+0xc7/0x1c0
>   __x64_sys_exit_group+0x2c/0x30
>   do_syscall_64+0x64/0x350
>   entry_SYSCALL_64_after_hwframe+0x44/0xa9
> 
> Fixes: bea890bdb161 ("vfio/pci: fix memory leaks in alloc_perm_bits()")
> Signed-off-by: Qian Cai <cai@....pw>
> ---
>  drivers/vfio/pci/vfio_pci_config.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)

I may get yelled at for it, but I need to break my next branch to fix
the lockdep issue you noted in my series, so I'm going to go ahead and
roll this into your previous patch.  Thanks,

Alex
 
> diff --git a/drivers/vfio/pci/vfio_pci_config.c b/drivers/vfio/pci/vfio_pci_config.c
> index d127a0c50940..8746c943247a 100644
> --- a/drivers/vfio/pci/vfio_pci_config.c
> +++ b/drivers/vfio/pci/vfio_pci_config.c
> @@ -1757,9 +1757,11 @@ void vfio_config_free(struct vfio_pci_device *vdev)
>  	vdev->vconfig = NULL;
>  	kfree(vdev->pci_config_map);
>  	vdev->pci_config_map = NULL;
> -	free_perm_bits(vdev->msi_perm);
> -	kfree(vdev->msi_perm);
> -	vdev->msi_perm = NULL;
> +	if (vdev->msi_perm) {
> +		free_perm_bits(vdev->msi_perm);
> +		kfree(vdev->msi_perm);
> +		vdev->msi_perm = NULL;
> +	}
>  }
>  
>  /*