All of lore.kernel.org
 help / color / mirror / Atom feed
From: Petr Vandrovec <petr@vandrovec.name>
To: <linux-nfs@vger.kernel.org>
Cc: <anna.schumaker@netapp.com>, <trond.myklebust@primarydata.com>
Subject: [PATCH] Do not dereference ERR_PTR(-EINVAL) in NFS client code
Date: Thu, 10 Nov 2016 12:42:24 -0800	[thread overview]
Message-ID: <20161110204224.xcbu3ttyk7codgqi@petr-dev3.eng.vmware.com> (raw)

Hi,
  Andy Adamson's change added code that unconditionally calls
rpc_clnt_xptr_switch_has_addr() with clp->cl_rpcclient from place
where clp->cl_rpcclient may be still set to its initial
ERR_PTR(-EINVAL) value, rather than to real RPC client.

This then causes dereference of
ERR_PTR(-EINVAL) + offsetof(rpc_clnt, cl_xpi.xpi_xpswitch)
in net/sunrpc/clnt.c at line 2773.

There seems to be other code that accesses cl_rpcclient, but
as far as I can tell, all remaining sites cannot encounter
clp that is in the middle of initializing.

Problem is also tracked in bugzilla.kernel.org as bug 182171.
As it is regression in 4.9.0, it would be great if it can be fixed
before 4.9.0 is final.

Thanks,
Petr Vandrovec



commit d2d51793392a176aaba7fe2ea3edb3c6f62d5e68
Author: Petr Vandrovec <petr@vandrovec.name>
Date:   Mon Nov 7 12:11:29 2016 -0800

    Ignore connections that have cl_rpcclient uninitialized
    
    cl_rpcclient starts as ERR_PTR(-EINVAL), and connections like that
    are floating freely through the system.  Most places check whether
    pointer is valid before dereferencing it, but newly added code
    in nfs_match_client does not.
    
    Which causes crashes when more than one NFS mount point is present.
    
    Signed-off-by: Petr Vandrovec <petr@vandrovec.name>

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index 7555ba8..ebecfb8 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -314,7 +314,8 @@ static struct nfs_client *nfs_match_client(const struct nfs_client_initdata *dat
 		/* Match the full socket address */
 		if (!rpc_cmp_addr_port(sap, clap))
 			/* Match all xprt_switch full socket addresses */
-			if (!rpc_clnt_xprt_switch_has_addr(clp->cl_rpcclient,
+			if (IS_ERR(clp->cl_rpcclient) ||
+                            !rpc_clnt_xprt_switch_has_addr(clp->cl_rpcclient,
 							   sap))
 				continue;
 

                 reply	other threads:[~2016-11-10 20:42 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161110204224.xcbu3ttyk7codgqi@petr-dev3.eng.vmware.com \
    --to=petr@vandrovec.name \
    --cc=anna.schumaker@netapp.com \
    --cc=linux-nfs@vger.kernel.org \
    --cc=trond.myklebust@primarydata.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.