linux-kernel - Re: [PATCH] NFS: check for allocation failure from mempool

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: <12d1e7a2ce5b0c64dfd81aeda75879c460e59fcb.camel@hammerspace.com>
Date:   Wed, 26 Feb 2020 23:48:47 +0000
From:   Trond Myklebust <trondmy@...merspace.com>
To:     "linux-nfs@...r.kernel.org" <linux-nfs@...r.kernel.org>,
        "colin.king@...onical.com" <colin.king@...onical.com>,
        "anna.schumaker@...app.com" <anna.schumaker@...app.com>
CC:     "kernel-janitors@...r.kernel.org" <kernel-janitors@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] NFS: check for allocation failure from mempool_alloc

On Wed, 2020-02-26 at 23:43 +0000, Colin King wrote:
> From: Colin Ian King <colin.king@...onical.com>
> 
> It is possible for mempool_alloc to return null when using
> the GFP_KERNEL flag, so return NULL and avoid a null pointer
> dereference on the following memset of the null pointer.

Umm, no. That would be a false positive by coverity.

If you look at the history of that function, you'll note that we
originally had those checks, but that Neil Brown removed them after
analysis of the mempool_alloc() function. He determined (correctly, I
believe) that any value that includes GFP_WAIT cannot fail to return a
valid pointer.

> 
> Addresses-Coverity: ("Dereference null return")
> Fixes: 2b17d725f9be ("NFS: Clean up writeback code")
> Signed-off-by: Colin Ian King <colin.king@...onical.com>
> ---
>  fs/nfs/write.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/fs/nfs/write.c b/fs/nfs/write.c
> index c478b772cc49..7ca036660dd1 100644
> --- a/fs/nfs/write.c
> +++ b/fs/nfs/write.c
> @@ -106,6 +106,9 @@ static struct nfs_pgio_header
> *nfs_writehdr_alloc(void)
>  {
>  	struct nfs_pgio_header *p = mempool_alloc(nfs_wdata_mempool,
> GFP_KERNEL);
>  
> +	if (!p)
> +		return NULL;
> +
>  	memset(p, 0, sizeof(*p));
>  	p->rw_mode = FMODE_WRITE;
>  	return p;
-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@...merspace.com