2016-11-10 20:42:31

by Petr Vandrovec

[permalink] [raw]
Subject: [PATCH] Do not dereference ERR_PTR(-EINVAL) in NFS client code

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 <[email protected]>
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 <[email protected]>

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;