linux-kernel - Re: general protection fault in encode_rpcb

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: <4B24FAE4-C7E8-4D01-9808-B8F4E9E59D64@gmail.com>
Date:   Tue, 8 May 2018 12:34:48 -0400
From:   Chuck Lever <chucklever@...il.com>
To:     Bruce Fields <bfields@...ldses.org>
Cc:     Trond Myklebust <trondmy@...mer.space>,
        "syzbot+4b98281f2401ab849f4b@...kaller.appspotmail.com" 
        <syzbot+4b98281f2401ab849f4b@...kaller.appspotmail.com>,
        "syzkaller-bugs@...glegroups.com" <syzkaller-bugs@...glegroups.com>,
        Anna Schumaker <anna.schumaker@...app.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Linux NFS Mailing List <linux-nfs@...r.kernel.org>,
        "jlayton@...nel.org" <jlayton@...nel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: general protection fault in encode_rpcb_string



> On May 8, 2018, at 12:15 PM, bfields@...ldses.org wrote:
> 
> On Tue, Apr 17, 2018 at 09:54:36PM +0000, Trond Myklebust wrote:
>> Yes, and we can probably convert it, and the other GFP_ATOMIC
>> allocations in the rpcbind client to use GFP_NOFS in order to improve
>> reliability.
> 
> Chuck, I think the GFP_ATOMIC is unnecessary here as well?
> 
> --b.
> 
> diff --git a/net/sunrpc/xprtrdma/rpc_rdma.c b/net/sunrpc/xprtrdma/rpc_rdma.c
> index e8adad33d0bb..de90c6c90cde 100644
> --- a/net/sunrpc/xprtrdma/rpc_rdma.c
> +++ b/net/sunrpc/xprtrdma/rpc_rdma.c
> @@ -228,7 +228,7 @@ rpcrdma_convert_iovs(struct rpcrdma_xprt *r_xprt, struct xdr_buf *xdrbuf,
> 			/* XXX: Certain upper layer operations do
> 			 *	not provide receive buffer pages.
> 			 */
> -			*ppages = alloc_page(GFP_ATOMIC);
> +			*ppages = alloc_page(GFP_NOFS);
> 			if (!*ppages)
> 				return -EAGAIN;
> 		}

This code can't sleep, as I understand it. Caller is holding
the transport write lock. This logic was copied from
xdr_partial_copy_from_skb, which uses GFP_ATOMIC.

Recall that this is here because of GETACL. As I've stated in
the past, the correct solution is to ensure that these pages
are provided in every case by the upper layer, making this
alloc_page call site unnecessary.


--
Chuck Lever
chucklever@...il.com