linux-kernel - Re: [PATCH] xen-blkback: prevent premature module unload

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20191211112754.GM980@Air-de-Roger>
Date:   Wed, 11 Dec 2019 12:28:44 +0100
From:   Roger Pau Monné <roger.pau@...rix.com>
To:     Paul Durrant <pdurrant@...zon.com>
CC:     <xen-devel@...ts.xenproject.org>, <linux-block@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>,
        Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>,
        Jens Axboe <axboe@...nel.dk>
Subject: Re: [PATCH] xen-blkback: prevent premature module unload

On Tue, Dec 10, 2019 at 02:53:05PM +0000, Paul Durrant wrote:
> Objects allocated by xen_blkif_alloc come from the 'blkif_cache' kmem
> cache. This cache is destoyed when xen-blkif is unloaded so it is
> necessary to wait for the deferred free routine used for such objects to
> complete. This necessity was missed in commit 14855954f636 "xen-blkback:
> allow module to be cleanly unloaded". This patch fixes the problem by
> taking/releasing extra module references in xen_blkif_alloc/free()
> respectively.
> 
> Signed-off-by: Paul Durrant <pdurrant@...zon.com>

Reviewed-by: Roger Pau Monné <roger.pau@...rix.com>

One nit below.

> ---
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
> Cc: "Roger Pau Monné" <roger.pau@...rix.com>
> Cc: Jens Axboe <axboe@...nel.dk>
> ---
>  drivers/block/xen-blkback/xenbus.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c
> index e8c5c54e1d26..59d576d27ca7 100644
> --- a/drivers/block/xen-blkback/xenbus.c
> +++ b/drivers/block/xen-blkback/xenbus.c
> @@ -171,6 +171,15 @@ static struct xen_blkif *xen_blkif_alloc(domid_t domid)
>  	blkif->domid = domid;
>  	atomic_set(&blkif->refcnt, 1);
>  	init_completion(&blkif->drain_complete);
> +
> +	/*
> +	 * Because freeing back to the cache may be deferred, it is not
> +	 * safe to unload the module (and hence destroy the cache) until
> +	 * this has completed. To prevent premature unloading, take an
> +	 * extra module reference here and release only when the object
> +	 * has been free back to the cache.
                    ^ freed
> +	 */
> +	__module_get(THIS_MODULE);
>  	INIT_WORK(&blkif->free_work, xen_blkif_deferred_free);
>  
>  	return blkif;
> @@ -320,6 +329,7 @@ static void xen_blkif_free(struct xen_blkif *blkif)
>  
>  	/* Make sure everything is drained before shutting down */
>  	kmem_cache_free(xen_blkif_cachep, blkif);
> +	module_put(THIS_MODULE);
>  }
>  
>  int __init xen_blkif_interface_init(void)
> -- 
> 2.20.1
>