linux-kernel - Re: [PATCH v2] pvcalls-front: fix potential null dereference

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, 15 Jan 2019 12:04:30 -0500
From:   Boris Ostrovsky <boris.ostrovsky@...cle.com>
To:     Wen Yang <wen.yang99@....com.cn>
Cc:     jgross@...e.com, sstabellini@...nel.org,
        xen-devel@...ts.xenproject.org, linux-kernel@...r.kernel.org,
        xue.zhihong@....com.cn, wang.yi59@....com.cn,
        Dan Carpenter <dan.carpenter@...cle.com>
Subject: Re: [PATCH v2] pvcalls-front: fix potential null dereference

On 1/14/19 9:31 PM, Wen Yang wrote:
>  static checker warning:
>     drivers/xen/pvcalls-front.c:373 alloc_active_ring()
>     error: we previously assumed 'map->active.ring' could be null
>            (see line 357)
>
> drivers/xen/pvcalls-front.c
>     351 static int alloc_active_ring(struct sock_mapping *map)
>     352 {
>     353     void *bytes;
>     354
>     355     map->active.ring = (struct pvcalls_data_intf *)
>     356         get_zeroed_page(GFP_KERNEL);
>     357     if (!map->active.ring)
>                     ^^^^^^^^^^^^^^^^^
> Check
>
>     358         goto out;
>     359
>     360     map->active.ring->ring_order = PVCALLS_RING_ORDER;
>     361     bytes = (void *)__get_free_pages(GFP_KERNEL | __GFP_ZERO,
>     362                     PVCALLS_RING_ORDER);
>     363     if (!bytes)
>     364         goto out;
>     365
>     366     map->active.data.in = bytes;
>     367     map->active.data.out = bytes +
>     368         XEN_FLEX_RING_SIZE(PVCALLS_RING_ORDER);
>     369
>     370     return 0;
>     371
>     372 out:
> --> 373     free_active_ring(map);
>                                  ^^^
> Add null check on map->active.ring before dereferencing it to avoid
> any NULL pointer dereferences.
>
> Fixes: 9f51c05dc41a ("pvcalls-front: Avoid get_free_pages(GFP_KERNEL)
> under spinlock")
> Reported-by: Dan Carpenter <dan.carpenter@...cle.com>
> Suggested-by: Boris Ostrovsky <boris.ostrovsky@...cle.com>
> Signed-off-by: Wen Yang <wen.yang99@....com.cn>
> CC: Boris Ostrovsky <boris.ostrovsky@...cle.com>
> CC: Juergen Gross <jgross@...e.com>
> CC: Stefano Stabellini <sstabellini@...nel.org>
> CC: Dan Carpenter <dan.carpenter@...cle.com>
> CC: xen-devel@...ts.xenproject.org
> CC: linux-kernel@...r.kernel.org


Reviewed-by: Boris Ostrovsky <boris.ostrovsky@...cle.com>

Applied to for-linus-4.21.


> ---
> v2->v1:
> - Add NULL check on map->active.ring and return
>   immediately if it is.
>
>  drivers/xen/pvcalls-front.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/xen/pvcalls-front.c b/drivers/xen/pvcalls-front.c
> index 307861f..8a249c9 100644
> --- a/drivers/xen/pvcalls-front.c
> +++ b/drivers/xen/pvcalls-front.c
> @@ -343,6 +343,9 @@ int pvcalls_front_socket(struct socket *sock)
>  
>  static void free_active_ring(struct sock_mapping *map)
>  {
> +	if (!map->active.ring)
> +		return;
> +
>  	free_pages((unsigned long)map->active.data.in,
>  			map->active.ring->ring_order);
>  	free_page((unsigned long)map->active.ring);