From 71afa85e798ebc81054164c947b189d163c92b30 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Sat, 16 Mar 2013 15:54:16 -0400 Subject: SUNRPC: Missing module alias for auth_rpcgss.ko Commit f344f6df "SUNRPC: Auto-load RPC authentication kernel modules", Mon Mar 20 13:44:08 2006, adds a request_module() call in rpcauth_create() to auto-load RPC security modules when a ULP tries to create a credential of that flavor. In rpcauth_create(), the name of the module to load is built like this: request_module("rpc-auth-%u", flavor); This means that for, say, RPC_AUTH_GSS, request_module() is looking for a module or alias called "rpc-auth-6". The GSS module is named "auth_rpcgss", and commit f344f6df does not add any new module aliases. There is also no such alias provided in /etc/modprobe.d on my system (Fedora 16). Without this alias, the GSS module is not loaded on demand. This is used by rpcauth_create(). The pseudoflavor_to_flavor() call can return RPC_AUTH_GSS, which is passed to request_module(). Signed-off-by: Chuck Lever Signed-off-by: Trond Myklebust --- net/sunrpc/auth_gss/auth_gss.c | 1 + 1 file changed, 1 insertion(+) diff --git a/net/sunrpc/auth_gss/auth_gss.c b/net/sunrpc/auth_gss/auth_gss.c index 5257d2982ba5..282dfb14db05 100644 --- a/net/sunrpc/auth_gss/auth_gss.c +++ b/net/sunrpc/auth_gss/auth_gss.c @@ -1733,6 +1733,7 @@ static void __exit exit_rpcsec_gss(void) rcu_barrier(); /* Wait for completion of call_rcu()'s */ } +MODULE_ALIAS("rpc-auth-6"); MODULE_LICENSE("GPL"); module_param_named(expired_cred_retry_delay, gss_expired_cred_retry_delay, -- cgit v1.2.2 From 72f4dc117b57e05120aaac6e218b8abc09a5c350 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Sat, 16 Mar 2013 15:54:25 -0400 Subject: NFS: Remove unneeded forward declaration I've built with NFSv4 enabled and disabled. This forward declaration does not seem to be required. Signed-off-by: Chuck Lever Signed-off-by: Trond Myklebust --- include/linux/nfs_xdr.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h index 90a4aa190b43..c1ca1f3f4935 100644 --- a/include/linux/nfs_xdr.h +++ b/include/linux/nfs_xdr.h @@ -14,9 +14,6 @@ #define NFS_DEF_FILE_IO_SIZE (4096U) #define NFS_MIN_FILE_IO_SIZE (1024U) -/* Forward declaration for NFS v3 */ -struct nfs4_secinfo_flavors; - struct nfs4_string { unsigned int len; char *data; -- cgit v1.2.2 From fb15b26f8ba3ff629a052faf3f4a4744585ca2dc Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Sat, 16 Mar 2013 15:54:34 -0400 Subject: SUNRPC: Define rpcsec_gss_info structure The NFSv4 SECINFO procedure returns a list of security flavors. Any GSS flavor also has a GSS tuple containing an OID, a quality-of- protection value, and a service value, which specifies a particular GSS pseudoflavor. For simplicity and efficiency, I'd like to return each GSS tuple from the NFSv4 SECINFO XDR decoder and pass it straight into the RPC client. Define a data structure that is visible to both the NFS client and the RPC client. Take structure and field names from the relevant standards to avoid confusion. Signed-off-by: Chuck Lever Signed-off-by: Trond Myklebust --- fs/nfs/nfs4namespace.c | 12 ++++++------ fs/nfs/nfs4xdr.c | 21 ++++++++++++--------- include/linux/nfs_xdr.h | 21 +++++---------------- include/linux/sunrpc/gss_api.h | 14 ++++++++++++-- net/sunrpc/auth_gss/gss_krb5_mech.c | 2 +- 5 files changed, 36 insertions(+), 34 deletions(-) diff --git a/fs/nfs/nfs4namespace.c b/fs/nfs/nfs4namespace.c index 0dd766079e1c..88231c92317c 100644 --- a/fs/nfs/nfs4namespace.c +++ b/fs/nfs/nfs4namespace.c @@ -138,23 +138,23 @@ rpc_authflavor_t nfs_find_best_sec(struct nfs4_secinfo_flavors *flavors) { struct gss_api_mech *mech; struct xdr_netobj oid; - int i; + unsigned int i; rpc_authflavor_t pseudoflavor = RPC_AUTH_UNIX; for (i = 0; i < flavors->num_flavors; i++) { - struct nfs4_secinfo_flavor *flavor; - flavor = &flavors->flavors[i]; + struct nfs4_secinfo4 *flavor = &flavors->flavors[i]; if (flavor->flavor == RPC_AUTH_NULL || flavor->flavor == RPC_AUTH_UNIX) { pseudoflavor = flavor->flavor; break; } else if (flavor->flavor == RPC_AUTH_GSS) { - oid.len = flavor->gss.sec_oid4.len; - oid.data = flavor->gss.sec_oid4.data; + oid.len = flavor->flavor_info.oid.len; + oid.data = flavor->flavor_info.oid.data; mech = gss_mech_get_by_OID(&oid); if (!mech) continue; - pseudoflavor = gss_svc_to_pseudoflavor(mech, flavor->gss.service); + pseudoflavor = gss_svc_to_pseudoflavor(mech, + flavor->flavor_info.service); gss_mech_put(mech); break; } diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c index 0b744895b9e1..a38fd179c34f 100644 --- a/fs/nfs/nfs4xdr.c +++ b/fs/nfs/nfs4xdr.c @@ -5205,27 +5205,30 @@ static int decode_delegreturn(struct xdr_stream *xdr) return decode_op_hdr(xdr, OP_DELEGRETURN); } -static int decode_secinfo_gss(struct xdr_stream *xdr, struct nfs4_secinfo_flavor *flavor) +static int decode_secinfo_gss(struct xdr_stream *xdr, + struct nfs4_secinfo4 *flavor) { + u32 oid_len; __be32 *p; p = xdr_inline_decode(xdr, 4); if (unlikely(!p)) goto out_overflow; - flavor->gss.sec_oid4.len = be32_to_cpup(p); - if (flavor->gss.sec_oid4.len > GSS_OID_MAX_LEN) + oid_len = be32_to_cpup(p); + if (oid_len > GSS_OID_MAX_LEN) goto out_err; - p = xdr_inline_decode(xdr, flavor->gss.sec_oid4.len); + p = xdr_inline_decode(xdr, oid_len); if (unlikely(!p)) goto out_overflow; - memcpy(flavor->gss.sec_oid4.data, p, flavor->gss.sec_oid4.len); + memcpy(flavor->flavor_info.oid.data, p, oid_len); + flavor->flavor_info.oid.len = oid_len; p = xdr_inline_decode(xdr, 8); if (unlikely(!p)) goto out_overflow; - flavor->gss.qop4 = be32_to_cpup(p++); - flavor->gss.service = be32_to_cpup(p); + flavor->flavor_info.qop = be32_to_cpup(p++); + flavor->flavor_info.service = be32_to_cpup(p); return 0; @@ -5238,10 +5241,10 @@ out_err: static int decode_secinfo_common(struct xdr_stream *xdr, struct nfs4_secinfo_res *res) { - struct nfs4_secinfo_flavor *sec_flavor; + struct nfs4_secinfo4 *sec_flavor; + unsigned int i, num_flavors; int status; __be32 *p; - int i, num_flavors; p = xdr_inline_decode(xdr, 4); if (unlikely(!p)) diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h index c1ca1f3f4935..b759467741eb 100644 --- a/include/linux/nfs_xdr.h +++ b/include/linux/nfs_xdr.h @@ -1049,25 +1049,14 @@ struct nfs4_fs_locations_res { struct nfs4_fs_locations *fs_locations; }; -struct nfs4_secinfo_oid { - unsigned int len; - char data[GSS_OID_MAX_LEN]; -}; - -struct nfs4_secinfo_gss { - struct nfs4_secinfo_oid sec_oid4; - unsigned int qop4; - unsigned int service; -}; - -struct nfs4_secinfo_flavor { - unsigned int flavor; - struct nfs4_secinfo_gss gss; +struct nfs4_secinfo4 { + u32 flavor; + struct rpcsec_gss_info flavor_info; }; struct nfs4_secinfo_flavors { - unsigned int num_flavors; - struct nfs4_secinfo_flavor flavors[0]; + unsigned int num_flavors; + struct nfs4_secinfo4 flavors[0]; }; struct nfs4_secinfo_arg { diff --git a/include/linux/sunrpc/gss_api.h b/include/linux/sunrpc/gss_api.h index a19e2547ae6a..98950e5a8877 100644 --- a/include/linux/sunrpc/gss_api.h +++ b/include/linux/sunrpc/gss_api.h @@ -25,10 +25,20 @@ struct gss_ctx { #define GSS_C_NO_BUFFER ((struct xdr_netobj) 0) #define GSS_C_NO_CONTEXT ((struct gss_ctx *) 0) -#define GSS_C_NULL_OID ((struct xdr_netobj) 0) /*XXX arbitrary length - is this set somewhere? */ #define GSS_OID_MAX_LEN 32 +struct rpcsec_gss_oid { + unsigned int len; + u8 data[GSS_OID_MAX_LEN]; +}; + +/* From RFC 3530 */ +struct rpcsec_gss_info { + struct rpcsec_gss_oid oid; + u32 qop; + u32 service; +}; /* gss-api prototypes; note that these are somewhat simplified versions of * the prototypes specified in RFC 2744. */ @@ -76,7 +86,7 @@ struct pf_desc { struct gss_api_mech { struct list_head gm_list; struct module *gm_owner; - struct xdr_netobj gm_oid; + struct rpcsec_gss_oid gm_oid; char *gm_name; const struct gss_api_ops *gm_ops; /* pseudoflavors supported by this mechanism: */ diff --git a/net/sunrpc/auth_gss/gss_krb5_mech.c b/net/sunrpc/auth_gss/gss_krb5_mech.c index d3611f11a8df..61d36ce3b366 100644 --- a/net/sunrpc/auth_gss/gss_krb5_mech.c +++ b/net/sunrpc/auth_gss/gss_krb5_mech.c @@ -754,7 +754,7 @@ MODULE_ALIAS("rpc-auth-gss-390005"); static struct gss_api_mech gss_kerberos_mech = { .gm_name = "krb5", .gm_owner = THIS_MODULE, - .gm_oid = {9, (void *)"\x2a\x86\x48\x86\xf7\x12\x01\x02\x02"}, + .gm_oid = { 9, "\x2a\x86\x48\x86\xf7\x12\x01\x02\x02" }, .gm_ops = &gss_kerberos_ops, .gm_pf_num = ARRAY_SIZE(gss_kerberos_pfs), .gm_pfs = gss_kerberos_pfs, -- cgit v1.2.2 From 9568c5e9a61de49f67f524404a27a1014a8d7f1e Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Sat, 16 Mar 2013 15:54:43 -0400 Subject: SUNRPC: Introduce rpcauth_get_pseudoflavor() A SECINFO reply may contain flavors whose kernel module is not yet loaded by the client's kernel. A new RPC client API, called rpcauth_get_pseudoflavor(), is introduced to do proper checking for support of a security flavor. When this API is invoked, the RPC client now tries to load the module for each flavor first before performing the "is this supported?" check. This means if a module is available on the client, but has not been loaded yet, it will be loaded and registered automatically when the SECINFO reply is processed. The new API can take a full GSS tuple (OID, QoP, and service). Previously only the OID and service were considered. nfs_find_best_sec() is updated to verify all flavors requested in a SECINFO reply, including AUTH_NULL and AUTH_UNIX. Previously these two flavors were simply assumed to be supported without consulting the RPC client. Note that the replaced version of nfs_find_best_sec() can return RPC_AUTH_MAXFLAVOR if the server returns a recognized OID but an unsupported "service" value. nfs_find_best_sec() now returns RPC_AUTH_UNIX in this case. Signed-off-by: Chuck Lever Signed-off-by: Trond Myklebust --- fs/nfs/nfs4namespace.c | 41 ++++++++++++++++++++--------------- include/linux/sunrpc/auth.h | 5 +++++ include/linux/sunrpc/gss_api.h | 5 ++--- net/sunrpc/auth.c | 35 ++++++++++++++++++++++++++++++ net/sunrpc/auth_gss/auth_gss.c | 1 + net/sunrpc/auth_gss/gss_mech_switch.c | 28 +++++++++++++++++++----- 6 files changed, 89 insertions(+), 26 deletions(-) diff --git a/fs/nfs/nfs4namespace.c b/fs/nfs/nfs4namespace.c index 88231c92317c..cdb0b41a4810 100644 --- a/fs/nfs/nfs4namespace.c +++ b/fs/nfs/nfs4namespace.c @@ -134,33 +134,38 @@ static size_t nfs_parse_server_name(char *string, size_t len, return ret; } +/** + * nfs_find_best_sec - Find a security mechanism supported locally + * @flavors: List of security tuples returned by SECINFO procedure + * + * Return the pseudoflavor of the first security mechanism in + * "flavors" that is locally supported. Return RPC_AUTH_UNIX if + * no matching flavor is found in the array. The "flavors" array + * is searched in the order returned from the server, per RFC 3530 + * recommendation. + */ rpc_authflavor_t nfs_find_best_sec(struct nfs4_secinfo_flavors *flavors) { - struct gss_api_mech *mech; - struct xdr_netobj oid; + rpc_authflavor_t pseudoflavor; + struct nfs4_secinfo4 *secinfo; unsigned int i; - rpc_authflavor_t pseudoflavor = RPC_AUTH_UNIX; for (i = 0; i < flavors->num_flavors; i++) { - struct nfs4_secinfo4 *flavor = &flavors->flavors[i]; - - if (flavor->flavor == RPC_AUTH_NULL || flavor->flavor == RPC_AUTH_UNIX) { - pseudoflavor = flavor->flavor; - break; - } else if (flavor->flavor == RPC_AUTH_GSS) { - oid.len = flavor->flavor_info.oid.len; - oid.data = flavor->flavor_info.oid.data; - mech = gss_mech_get_by_OID(&oid); - if (!mech) - continue; - pseudoflavor = gss_svc_to_pseudoflavor(mech, - flavor->flavor_info.service); - gss_mech_put(mech); + secinfo = &flavors->flavors[i]; + + switch (secinfo->flavor) { + case RPC_AUTH_NULL: + case RPC_AUTH_UNIX: + case RPC_AUTH_GSS: + pseudoflavor = rpcauth_get_pseudoflavor(secinfo->flavor, + &secinfo->flavor_info); + if (pseudoflavor != RPC_AUTH_MAXFLAVOR) + return pseudoflavor; break; } } - return pseudoflavor; + return RPC_AUTH_UNIX; } static rpc_authflavor_t nfs4_negotiate_security(struct inode *inode, struct qstr *name) diff --git a/include/linux/sunrpc/auth.h b/include/linux/sunrpc/auth.h index 58fda1c3c783..6851da4cb416 100644 --- a/include/linux/sunrpc/auth.h +++ b/include/linux/sunrpc/auth.h @@ -22,6 +22,8 @@ /* size of the nodename buffer */ #define UNX_MAXNODENAME 32 +struct rpcsec_gss_info; + /* Work around the lack of a VFS credential */ struct auth_cred { kuid_t uid; @@ -103,6 +105,7 @@ struct rpc_authops { int (*pipes_create)(struct rpc_auth *); void (*pipes_destroy)(struct rpc_auth *); int (*list_pseudoflavors)(rpc_authflavor_t *, int); + rpc_authflavor_t (*info2flavor)(struct rpcsec_gss_info *); }; struct rpc_credops { @@ -137,6 +140,8 @@ int rpcauth_register(const struct rpc_authops *); int rpcauth_unregister(const struct rpc_authops *); struct rpc_auth * rpcauth_create(rpc_authflavor_t, struct rpc_clnt *); void rpcauth_release(struct rpc_auth *); +rpc_authflavor_t rpcauth_get_pseudoflavor(rpc_authflavor_t, + struct rpcsec_gss_info *); int rpcauth_list_flavors(rpc_authflavor_t *, int); struct rpc_cred * rpcauth_lookup_credcache(struct rpc_auth *, struct auth_cred *, int); void rpcauth_init_cred(struct rpc_cred *, const struct auth_cred *, struct rpc_auth *, const struct rpc_credops *); diff --git a/include/linux/sunrpc/gss_api.h b/include/linux/sunrpc/gss_api.h index 98950e5a8877..aba7687ca884 100644 --- a/include/linux/sunrpc/gss_api.h +++ b/include/linux/sunrpc/gss_api.h @@ -127,9 +127,8 @@ struct gss_api_ops { int gss_mech_register(struct gss_api_mech *); void gss_mech_unregister(struct gss_api_mech *); -/* returns a mechanism descriptor given an OID, and increments the mechanism's - * reference count. */ -struct gss_api_mech * gss_mech_get_by_OID(struct xdr_netobj *); +/* Given a GSS security tuple, look up a pseudoflavor */ +rpc_authflavor_t gss_mech_info2flavor(struct rpcsec_gss_info *); /* Returns a reference to a mechanism, given a name like "krb5" etc. */ struct gss_api_mech *gss_mech_get_by_name(const char *); diff --git a/net/sunrpc/auth.c b/net/sunrpc/auth.c index f5294047df77..9b81be8d9946 100644 --- a/net/sunrpc/auth.c +++ b/net/sunrpc/auth.c @@ -123,6 +123,41 @@ rpcauth_unregister(const struct rpc_authops *ops) } EXPORT_SYMBOL_GPL(rpcauth_unregister); +/** + * rpcauth_get_pseudoflavor - check if security flavor is supported + * @flavor: a security flavor + * @info: a GSS mech OID, quality of protection, and service value + * + * Verifies that an appropriate kernel module is available or already loaded. + * Returns an equivalent pseudoflavor, or RPC_AUTH_MAXFLAVOR if "flavor" is + * not supported locally. + */ +rpc_authflavor_t +rpcauth_get_pseudoflavor(rpc_authflavor_t flavor, struct rpcsec_gss_info *info) +{ + const struct rpc_authops *ops; + rpc_authflavor_t pseudoflavor; + + ops = auth_flavors[flavor]; + if (ops == NULL) + request_module("rpc-auth-%u", flavor); + spin_lock(&rpc_authflavor_lock); + ops = auth_flavors[flavor]; + if (ops == NULL || !try_module_get(ops->owner)) { + spin_unlock(&rpc_authflavor_lock); + return RPC_AUTH_MAXFLAVOR; + } + spin_unlock(&rpc_authflavor_lock); + + pseudoflavor = flavor; + if (ops->info2flavor != NULL) + pseudoflavor = ops->info2flavor(info); + + module_put(ops->owner); + return pseudoflavor; +} +EXPORT_SYMBOL_GPL(rpcauth_get_pseudoflavor); + /** * rpcauth_list_flavors - discover registered flavors and pseudoflavors * @array: array to fill in diff --git a/net/sunrpc/auth_gss/auth_gss.c b/net/sunrpc/auth_gss/auth_gss.c index 282dfb14db05..a7420076ef39 100644 --- a/net/sunrpc/auth_gss/auth_gss.c +++ b/net/sunrpc/auth_gss/auth_gss.c @@ -1641,6 +1641,7 @@ static const struct rpc_authops authgss_ops = { .pipes_create = gss_pipes_dentries_create, .pipes_destroy = gss_pipes_dentries_destroy, .list_pseudoflavors = gss_mech_list_pseudoflavors, + .info2flavor = gss_mech_info2flavor, }; static const struct rpc_credops gss_credops = { diff --git a/net/sunrpc/auth_gss/gss_mech_switch.c b/net/sunrpc/auth_gss/gss_mech_switch.c index f0f4eee63a35..4db66f5f490e 100644 --- a/net/sunrpc/auth_gss/gss_mech_switch.c +++ b/net/sunrpc/auth_gss/gss_mech_switch.c @@ -171,8 +171,7 @@ struct gss_api_mech * gss_mech_get_by_name(const char *name) } EXPORT_SYMBOL_GPL(gss_mech_get_by_name); -struct gss_api_mech * -gss_mech_get_by_OID(struct xdr_netobj *obj) +static struct gss_api_mech *gss_mech_get_by_OID(struct rpcsec_gss_oid *obj) { struct gss_api_mech *pos, *gm = NULL; @@ -188,11 +187,8 @@ gss_mech_get_by_OID(struct xdr_netobj *obj) } spin_unlock(®istered_mechs_lock); return gm; - } -EXPORT_SYMBOL_GPL(gss_mech_get_by_OID); - static inline int mech_supports_pseudoflavor(struct gss_api_mech *gm, u32 pseudoflavor) { @@ -282,6 +278,28 @@ gss_svc_to_pseudoflavor(struct gss_api_mech *gm, u32 service) } EXPORT_SYMBOL_GPL(gss_svc_to_pseudoflavor); +/** + * gss_mech_info2flavor - look up a pseudoflavor given a GSS tuple + * @info: a GSS mech OID, quality of protection, and service value + * + * Returns a matching pseudoflavor, or RPC_AUTH_MAXFLAVOR if the tuple is + * not supported. + */ +rpc_authflavor_t gss_mech_info2flavor(struct rpcsec_gss_info *info) +{ + rpc_authflavor_t pseudoflavor; + struct gss_api_mech *gm; + + gm = gss_mech_get_by_OID(&info->oid); + if (gm == NULL) + return RPC_AUTH_MAXFLAVOR; + + pseudoflavor = gss_svc_to_pseudoflavor(gm, info->service); + + gss_mech_put(gm); + return pseudoflavor; +} + u32 gss_pseudoflavor_to_service(struct gss_api_mech *gm, u32 pseudoflavor) { -- cgit v1.2.2 From f783288f0cf47f1988f5284c89e325bd22c74f91 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Sat, 16 Mar 2013 15:54:52 -0400 Subject: SUNRPC: Load GSS kernel module by OID The current GSS mech switch can find and load GSS pseudoflavor modules by name ("krb5") or pseudoflavor number ("390003"), but cannot find GSS modules by GSS tuple: [ "1.2.840.113554.1.2.2", GSS_C_QOP_DEFAULT, RPC_GSS_SVC_NONE ] This is important when dealing with a SECINFO request. A SECINFO reply contains a list of flavors the server supports for the requested export, but GSS flavors also have a GSS tuple that maps to a pseudoflavor (like 390003 for krb5). If the GSS module that supports the OID in the tuple is not loaded, our client is not able to load that module dynamically to support that pseudoflavor. Add a way for the GSS mech switch to load GSS pseudoflavor support by OID before searching for the pseudoflavor that matches the OID and service. Signed-off-by: Chuck Lever Cc: David Howells Signed-off-by: Trond Myklebust --- net/sunrpc/Kconfig | 1 + net/sunrpc/auth_gss/gss_krb5_mech.c | 1 + net/sunrpc/auth_gss/gss_mech_switch.c | 7 +++++++ 3 files changed, 9 insertions(+) diff --git a/net/sunrpc/Kconfig b/net/sunrpc/Kconfig index 516fe2caac2c..804f4f623ca4 100644 --- a/net/sunrpc/Kconfig +++ b/net/sunrpc/Kconfig @@ -3,6 +3,7 @@ config SUNRPC config SUNRPC_GSS tristate + select OID_REGISTRY config SUNRPC_BACKCHANNEL bool diff --git a/net/sunrpc/auth_gss/gss_krb5_mech.c b/net/sunrpc/auth_gss/gss_krb5_mech.c index 61d36ce3b366..b822ec5cdc58 100644 --- a/net/sunrpc/auth_gss/gss_krb5_mech.c +++ b/net/sunrpc/auth_gss/gss_krb5_mech.c @@ -750,6 +750,7 @@ MODULE_ALIAS("rpc-auth-gss-krb5p"); MODULE_ALIAS("rpc-auth-gss-390003"); MODULE_ALIAS("rpc-auth-gss-390004"); MODULE_ALIAS("rpc-auth-gss-390005"); +MODULE_ALIAS("rpc-auth-gss-1.2.840.113554.1.2.2"); static struct gss_api_mech gss_kerberos_mech = { .gm_name = "krb5", diff --git a/net/sunrpc/auth_gss/gss_mech_switch.c b/net/sunrpc/auth_gss/gss_mech_switch.c index 4db66f5f490e..92a72404e6d5 100644 --- a/net/sunrpc/auth_gss/gss_mech_switch.c +++ b/net/sunrpc/auth_gss/gss_mech_switch.c @@ -36,6 +36,7 @@ #include #include #include +#include #include #include #include @@ -174,6 +175,12 @@ EXPORT_SYMBOL_GPL(gss_mech_get_by_name); static struct gss_api_mech *gss_mech_get_by_OID(struct rpcsec_gss_oid *obj) { struct gss_api_mech *pos, *gm = NULL; + char buf[32]; + + if (sprint_oid(obj->data, obj->len, buf, sizeof(buf)) < 0) + return NULL; + dprintk("RPC: %s(%s)\n", __func__, buf); + request_module("rpc-auth-gss-%s", buf); spin_lock(®istered_mechs_lock); list_for_each_entry(pos, ®istered_mechs, gm_list) { -- cgit v1.2.2 From 83523d083a045a2069e5f3443d2e4f810a6e6d9a Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Sat, 16 Mar 2013 15:55:01 -0400 Subject: SUNRPC: Consider qop when looking up pseudoflavors The NFSv4 SECINFO operation returns a list of security flavors that the server supports for a particular share. An NFSv4 client is supposed to pick a pseudoflavor it supports that corresponds to one of the flavors returned by the server. GSS flavors in this list have a GSS tuple that identify a specific GSS pseudoflavor. Currently our client ignores the GSS tuple's "qop" value. A matching pseudoflavor is chosen based only on the OID and service value. So far this omission has not had much effect on Linux. The NFSv4 protocol currently supports only one qop value: GSS_C_QOP_DEFAULT, also known as zero. However, if an NFSv4 server happens to return something other than zero in the qop field, our client won't notice. This could cause the client to behave in incorrect ways that could have security implications. Signed-off-by: Chuck Lever Signed-off-by: Trond Myklebust --- include/linux/sunrpc/gss_api.h | 5 ++++- net/sunrpc/auth_gss/gss_krb5_mech.c | 3 +++ net/sunrpc/auth_gss/gss_mech_switch.c | 20 ++++++++++++++------ net/sunrpc/auth_gss/svcauth_gss.c | 4 +++- 4 files changed, 24 insertions(+), 8 deletions(-) diff --git a/include/linux/sunrpc/gss_api.h b/include/linux/sunrpc/gss_api.h index aba7687ca884..96e5a81a54d7 100644 --- a/include/linux/sunrpc/gss_api.h +++ b/include/linux/sunrpc/gss_api.h @@ -25,6 +25,7 @@ struct gss_ctx { #define GSS_C_NO_BUFFER ((struct xdr_netobj) 0) #define GSS_C_NO_CONTEXT ((struct gss_ctx *) 0) +#define GSS_C_QOP_DEFAULT (0) /*XXX arbitrary length - is this set somewhere? */ #define GSS_OID_MAX_LEN 32 @@ -68,12 +69,14 @@ u32 gss_unwrap( u32 gss_delete_sec_context( struct gss_ctx **ctx_id); -u32 gss_svc_to_pseudoflavor(struct gss_api_mech *, u32 service); +rpc_authflavor_t gss_svc_to_pseudoflavor(struct gss_api_mech *, u32 qop, + u32 service); u32 gss_pseudoflavor_to_service(struct gss_api_mech *, u32 pseudoflavor); char *gss_service_to_auth_domain_name(struct gss_api_mech *, u32 service); struct pf_desc { u32 pseudoflavor; + u32 qop; u32 service; char *name; char *auth_domain_name; diff --git a/net/sunrpc/auth_gss/gss_krb5_mech.c b/net/sunrpc/auth_gss/gss_krb5_mech.c index b822ec5cdc58..33255ff889c0 100644 --- a/net/sunrpc/auth_gss/gss_krb5_mech.c +++ b/net/sunrpc/auth_gss/gss_krb5_mech.c @@ -729,16 +729,19 @@ static const struct gss_api_ops gss_kerberos_ops = { static struct pf_desc gss_kerberos_pfs[] = { [0] = { .pseudoflavor = RPC_AUTH_GSS_KRB5, + .qop = GSS_C_QOP_DEFAULT, .service = RPC_GSS_SVC_NONE, .name = "krb5", }, [1] = { .pseudoflavor = RPC_AUTH_GSS_KRB5I, + .qop = GSS_C_QOP_DEFAULT, .service = RPC_GSS_SVC_INTEGRITY, .name = "krb5i", }, [2] = { .pseudoflavor = RPC_AUTH_GSS_KRB5P, + .qop = GSS_C_QOP_DEFAULT, .service = RPC_GSS_SVC_PRIVACY, .name = "krb5p", }, diff --git a/net/sunrpc/auth_gss/gss_mech_switch.c b/net/sunrpc/auth_gss/gss_mech_switch.c index 92a72404e6d5..81fb6f3e2424 100644 --- a/net/sunrpc/auth_gss/gss_mech_switch.c +++ b/net/sunrpc/auth_gss/gss_mech_switch.c @@ -271,19 +271,27 @@ int gss_mech_list_pseudoflavors(rpc_authflavor_t *array_ptr, int size) return i; } -u32 -gss_svc_to_pseudoflavor(struct gss_api_mech *gm, u32 service) +/** + * gss_svc_to_pseudoflavor - map a GSS service number to a pseudoflavor + * @gm: GSS mechanism handle + * @qop: GSS quality-of-protection value + * @service: GSS service value + * + * Returns a matching security flavor, or RPC_AUTH_MAXFLAVOR if none is found. + */ +rpc_authflavor_t gss_svc_to_pseudoflavor(struct gss_api_mech *gm, u32 qop, + u32 service) { int i; for (i = 0; i < gm->gm_pf_num; i++) { - if (gm->gm_pfs[i].service == service) { + if (gm->gm_pfs[i].qop == qop && + gm->gm_pfs[i].service == service) { return gm->gm_pfs[i].pseudoflavor; } } - return RPC_AUTH_MAXFLAVOR; /* illegal value */ + return RPC_AUTH_MAXFLAVOR; } -EXPORT_SYMBOL_GPL(gss_svc_to_pseudoflavor); /** * gss_mech_info2flavor - look up a pseudoflavor given a GSS tuple @@ -301,7 +309,7 @@ rpc_authflavor_t gss_mech_info2flavor(struct rpcsec_gss_info *info) if (gm == NULL) return RPC_AUTH_MAXFLAVOR; - pseudoflavor = gss_svc_to_pseudoflavor(gm, info->service); + pseudoflavor = gss_svc_to_pseudoflavor(gm, info->qop, info->service); gss_mech_put(gm); return pseudoflavor; diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c index f7d34e7b6f81..74f6d30f5ded 100644 --- a/net/sunrpc/auth_gss/svcauth_gss.c +++ b/net/sunrpc/auth_gss/svcauth_gss.c @@ -1216,7 +1216,9 @@ svcauth_gss_accept(struct svc_rqst *rqstp, __be32 *authp) svcdata->rsci = rsci; cache_get(&rsci->h); rqstp->rq_cred.cr_flavor = gss_svc_to_pseudoflavor( - rsci->mechctx->mech_type, gc->gc_svc); + rsci->mechctx->mech_type, + GSS_C_QOP_DEFAULT, + gc->gc_svc); ret = SVC_OK; goto out; } -- cgit v1.2.2 From a77c806fb9d097bb7733b64207cf52fc2c6438bb Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Sat, 16 Mar 2013 15:55:10 -0400 Subject: SUNRPC: Refactor nfsd4_do_encode_secinfo() Clean up. This matches a similar API for the client side, and keeps ULP fingers out the of the GSS mech switch. Signed-off-by: Chuck Lever Acked-by: J. Bruce Fields Signed-off-by: Trond Myklebust --- fs/nfsd/nfs4xdr.c | 24 +++++++++++------------- include/linux/sunrpc/auth.h | 4 ++++ include/linux/sunrpc/gss_api.h | 3 +++ net/sunrpc/auth.c | 35 +++++++++++++++++++++++++++++++++++ net/sunrpc/auth_gss/auth_gss.c | 1 + net/sunrpc/auth_gss/gss_mech_switch.c | 35 +++++++++++++++++++++++++++++++++-- 6 files changed, 87 insertions(+), 15 deletions(-) diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c index 01168865dd37..2a2745615b42 100644 --- a/fs/nfsd/nfs4xdr.c +++ b/fs/nfsd/nfs4xdr.c @@ -3138,10 +3138,9 @@ nfsd4_encode_rename(struct nfsd4_compoundres *resp, __be32 nfserr, struct nfsd4_ static __be32 nfsd4_do_encode_secinfo(struct nfsd4_compoundres *resp, - __be32 nfserr,struct svc_export *exp) + __be32 nfserr, struct svc_export *exp) { - int i = 0; - u32 nflavs; + u32 i, nflavs; struct exp_flavor_info *flavs; struct exp_flavor_info def_flavs[2]; __be32 *p; @@ -3172,30 +3171,29 @@ nfsd4_do_encode_secinfo(struct nfsd4_compoundres *resp, WRITE32(nflavs); ADJUST_ARGS(); for (i = 0; i < nflavs; i++) { - u32 flav = flavs[i].pseudoflavor; - struct gss_api_mech *gm = gss_mech_get_by_pseudoflavor(flav); + struct rpcsec_gss_info info; - if (gm) { + if (rpcauth_get_gssinfo(flavs[i].pseudoflavor, &info) == 0) { RESERVE_SPACE(4); WRITE32(RPC_AUTH_GSS); ADJUST_ARGS(); - RESERVE_SPACE(4 + gm->gm_oid.len); - WRITE32(gm->gm_oid.len); - WRITEMEM(gm->gm_oid.data, gm->gm_oid.len); + RESERVE_SPACE(4 + info.oid.len); + WRITE32(info.oid.len); + WRITEMEM(info.oid.data, info.oid.len); ADJUST_ARGS(); RESERVE_SPACE(4); - WRITE32(0); /* qop */ + WRITE32(info.qop); ADJUST_ARGS(); RESERVE_SPACE(4); - WRITE32(gss_pseudoflavor_to_service(gm, flav)); + WRITE32(info.service); ADJUST_ARGS(); - gss_mech_put(gm); } else { RESERVE_SPACE(4); - WRITE32(flav); + WRITE32(flavs[i].pseudoflavor); ADJUST_ARGS(); } } + out: if (exp) exp_put(exp); diff --git a/include/linux/sunrpc/auth.h b/include/linux/sunrpc/auth.h index 6851da4cb416..0dd00f4f6810 100644 --- a/include/linux/sunrpc/auth.h +++ b/include/linux/sunrpc/auth.h @@ -106,6 +106,8 @@ struct rpc_authops { void (*pipes_destroy)(struct rpc_auth *); int (*list_pseudoflavors)(rpc_authflavor_t *, int); rpc_authflavor_t (*info2flavor)(struct rpcsec_gss_info *); + int (*flavor2info)(rpc_authflavor_t, + struct rpcsec_gss_info *); }; struct rpc_credops { @@ -142,6 +144,8 @@ struct rpc_auth * rpcauth_create(rpc_authflavor_t, struct rpc_clnt *); void rpcauth_release(struct rpc_auth *); rpc_authflavor_t rpcauth_get_pseudoflavor(rpc_authflavor_t, struct rpcsec_gss_info *); +int rpcauth_get_gssinfo(rpc_authflavor_t, + struct rpcsec_gss_info *); int rpcauth_list_flavors(rpc_authflavor_t *, int); struct rpc_cred * rpcauth_lookup_credcache(struct rpc_auth *, struct auth_cred *, int); void rpcauth_init_cred(struct rpc_cred *, const struct auth_cred *, struct rpc_auth *, const struct rpc_credops *); diff --git a/include/linux/sunrpc/gss_api.h b/include/linux/sunrpc/gss_api.h index 96e5a81a54d7..fca23380e667 100644 --- a/include/linux/sunrpc/gss_api.h +++ b/include/linux/sunrpc/gss_api.h @@ -133,6 +133,9 @@ void gss_mech_unregister(struct gss_api_mech *); /* Given a GSS security tuple, look up a pseudoflavor */ rpc_authflavor_t gss_mech_info2flavor(struct rpcsec_gss_info *); +/* Given a pseudoflavor, look up a GSS security tuple */ +int gss_mech_flavor2info(rpc_authflavor_t, struct rpcsec_gss_info *); + /* Returns a reference to a mechanism, given a name like "krb5" etc. */ struct gss_api_mech *gss_mech_get_by_name(const char *); diff --git a/net/sunrpc/auth.c b/net/sunrpc/auth.c index 9b81be8d9946..2bc0cc2196e0 100644 --- a/net/sunrpc/auth.c +++ b/net/sunrpc/auth.c @@ -158,6 +158,41 @@ rpcauth_get_pseudoflavor(rpc_authflavor_t flavor, struct rpcsec_gss_info *info) } EXPORT_SYMBOL_GPL(rpcauth_get_pseudoflavor); +/** + * rpcauth_get_gssinfo - find GSS tuple matching a GSS pseudoflavor + * @pseudoflavor: GSS pseudoflavor to match + * @info: rpcsec_gss_info structure to fill in + * + * Returns zero and fills in "info" if pseudoflavor matches a + * supported mechanism. + */ +int +rpcauth_get_gssinfo(rpc_authflavor_t pseudoflavor, struct rpcsec_gss_info *info) +{ + rpc_authflavor_t flavor = pseudoflavor_to_flavor(pseudoflavor); + const struct rpc_authops *ops; + int result; + + ops = auth_flavors[flavor]; + if (ops == NULL) + request_module("rpc-auth-%u", flavor); + spin_lock(&rpc_authflavor_lock); + ops = auth_flavors[flavor]; + if (ops == NULL || !try_module_get(ops->owner)) { + spin_unlock(&rpc_authflavor_lock); + return -ENOENT; + } + spin_unlock(&rpc_authflavor_lock); + + result = -ENOENT; + if (ops->flavor2info != NULL) + result = ops->flavor2info(pseudoflavor, info); + + module_put(ops->owner); + return result; +} +EXPORT_SYMBOL_GPL(rpcauth_get_gssinfo); + /** * rpcauth_list_flavors - discover registered flavors and pseudoflavors * @array: array to fill in diff --git a/net/sunrpc/auth_gss/auth_gss.c b/net/sunrpc/auth_gss/auth_gss.c index a7420076ef39..51415b07174e 100644 --- a/net/sunrpc/auth_gss/auth_gss.c +++ b/net/sunrpc/auth_gss/auth_gss.c @@ -1642,6 +1642,7 @@ static const struct rpc_authops authgss_ops = { .pipes_destroy = gss_pipes_dentries_destroy, .list_pseudoflavors = gss_mech_list_pseudoflavors, .info2flavor = gss_mech_info2flavor, + .flavor2info = gss_mech_flavor2info, }; static const struct rpc_credops gss_credops = { diff --git a/net/sunrpc/auth_gss/gss_mech_switch.c b/net/sunrpc/auth_gss/gss_mech_switch.c index 81fb6f3e2424..deaa7ae81cdf 100644 --- a/net/sunrpc/auth_gss/gss_mech_switch.c +++ b/net/sunrpc/auth_gss/gss_mech_switch.c @@ -240,8 +240,6 @@ gss_mech_get_by_pseudoflavor(u32 pseudoflavor) return gm; } -EXPORT_SYMBOL_GPL(gss_mech_get_by_pseudoflavor); - /** * gss_mech_list_pseudoflavors - Discover registered GSS pseudoflavors * @array: array to fill in @@ -315,6 +313,39 @@ rpc_authflavor_t gss_mech_info2flavor(struct rpcsec_gss_info *info) return pseudoflavor; } +/** + * gss_mech_flavor2info - look up a GSS tuple for a given pseudoflavor + * @pseudoflavor: GSS pseudoflavor to match + * @info: rpcsec_gss_info structure to fill in + * + * Returns zero and fills in "info" if pseudoflavor matches a + * supported mechanism. Otherwise a negative errno is returned. + */ +int gss_mech_flavor2info(rpc_authflavor_t pseudoflavor, + struct rpcsec_gss_info *info) +{ + struct gss_api_mech *gm; + int i; + + gm = gss_mech_get_by_pseudoflavor(pseudoflavor); + if (gm == NULL) + return -ENOENT; + + for (i = 0; i < gm->gm_pf_num; i++) { + if (gm->gm_pfs[i].pseudoflavor == pseudoflavor) { + memcpy(info->oid.data, gm->gm_oid.data, gm->gm_oid.len); + info->oid.len = gm->gm_oid.len; + info->qop = gm->gm_pfs[i].qop; + info->service = gm->gm_pfs[i].service; + gss_mech_put(gm); + return 0; + } + } + + gss_mech_put(gm); + return -ENOENT; +} + u32 gss_pseudoflavor_to_service(struct gss_api_mech *gm, u32 pseudoflavor) { -- cgit v1.2.2 From 6599c0acae10e929b5315821c1d064cd13fe7648 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Sat, 16 Mar 2013 15:55:19 -0400 Subject: SUNRPC: Make gss_mech_get() static gss_mech_get() is no longer used outside of gss_mech_switch.c. Signed-off-by: Chuck Lever Signed-off-by: Trond Myklebust --- include/linux/sunrpc/gss_api.h | 3 --- net/sunrpc/auth_gss/gss_mech_switch.c | 5 +---- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/include/linux/sunrpc/gss_api.h b/include/linux/sunrpc/gss_api.h index fca23380e667..f32b7a47e13f 100644 --- a/include/linux/sunrpc/gss_api.h +++ b/include/linux/sunrpc/gss_api.h @@ -145,9 +145,6 @@ struct gss_api_mech *gss_mech_get_by_pseudoflavor(u32); /* Fill in an array with a list of supported pseudoflavors */ int gss_mech_list_pseudoflavors(rpc_authflavor_t *, int); -/* Just increments the mechanism's reference count and returns its input: */ -struct gss_api_mech * gss_mech_get(struct gss_api_mech *); - /* For every successful gss_mech_get or gss_mech_get_by_* call there must be a * corresponding call to gss_mech_put. */ void gss_mech_put(struct gss_api_mech *); diff --git a/net/sunrpc/auth_gss/gss_mech_switch.c b/net/sunrpc/auth_gss/gss_mech_switch.c index deaa7ae81cdf..89416522ef79 100644 --- a/net/sunrpc/auth_gss/gss_mech_switch.c +++ b/net/sunrpc/auth_gss/gss_mech_switch.c @@ -132,15 +132,12 @@ gss_mech_unregister(struct gss_api_mech *gm) EXPORT_SYMBOL_GPL(gss_mech_unregister); -struct gss_api_mech * -gss_mech_get(struct gss_api_mech *gm) +static struct gss_api_mech *gss_mech_get(struct gss_api_mech *gm) { __module_get(gm->gm_owner); return gm; } -EXPORT_SYMBOL_GPL(gss_mech_get); - static struct gss_api_mech * _gss_mech_get_by_name(const char *name) { -- cgit v1.2.2 From 5007220b87d2ae9da1ab2025bfa35f6e2b3376f5 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Sat, 16 Mar 2013 15:55:27 -0400 Subject: SUNRPC: Remove EXPORT_SYMBOL_GPL() from GSS mech switch Clean up: Reduce the symbol table footprint for auth_rpcgss.ko by removing exported symbols for functions that are no longer used outside of auth_rpcgss.ko. The remaining two EXPORTs in gss_mech_switch.c get documenting comments. Signed-off-by: Chuck Lever Signed-off-by: Trond Myklebust --- net/sunrpc/auth_gss/gss_mech_switch.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/net/sunrpc/auth_gss/gss_mech_switch.c b/net/sunrpc/auth_gss/gss_mech_switch.c index 89416522ef79..79881d6e68a1 100644 --- a/net/sunrpc/auth_gss/gss_mech_switch.c +++ b/net/sunrpc/auth_gss/gss_mech_switch.c @@ -103,8 +103,13 @@ out: return status; } -int -gss_mech_register(struct gss_api_mech *gm) +/** + * gss_mech_register - register a GSS mechanism + * @gm: GSS mechanism handle + * + * Returns zero if successful, or a negative errno. + */ +int gss_mech_register(struct gss_api_mech *gm) { int status; @@ -117,11 +122,14 @@ gss_mech_register(struct gss_api_mech *gm) dprintk("RPC: registered gss mechanism %s\n", gm->gm_name); return 0; } - EXPORT_SYMBOL_GPL(gss_mech_register); -void -gss_mech_unregister(struct gss_api_mech *gm) +/** + * gss_mech_unregister - release a GSS mechanism + * @gm: GSS mechanism handle + * + */ +void gss_mech_unregister(struct gss_api_mech *gm) { spin_lock(®istered_mechs_lock); list_del(&gm->gm_list); @@ -129,7 +137,6 @@ gss_mech_unregister(struct gss_api_mech *gm) dprintk("RPC: unregistered gss mechanism %s\n", gm->gm_name); gss_mech_free(gm); } - EXPORT_SYMBOL_GPL(gss_mech_unregister); static struct gss_api_mech *gss_mech_get(struct gss_api_mech *gm) @@ -167,7 +174,6 @@ struct gss_api_mech * gss_mech_get_by_name(const char *name) } return gm; } -EXPORT_SYMBOL_GPL(gss_mech_get_by_name); static struct gss_api_mech *gss_mech_get_by_OID(struct rpcsec_gss_oid *obj) { @@ -355,8 +361,6 @@ gss_pseudoflavor_to_service(struct gss_api_mech *gm, u32 pseudoflavor) return 0; } -EXPORT_SYMBOL_GPL(gss_pseudoflavor_to_service); - char * gss_service_to_auth_domain_name(struct gss_api_mech *gm, u32 service) { @@ -369,8 +373,6 @@ gss_service_to_auth_domain_name(struct gss_api_mech *gm, u32 service) return NULL; } -EXPORT_SYMBOL_GPL(gss_service_to_auth_domain_name); - void gss_mech_put(struct gss_api_mech * gm) { @@ -378,8 +380,6 @@ gss_mech_put(struct gss_api_mech * gm) module_put(gm->gm_owner); } -EXPORT_SYMBOL_GPL(gss_mech_put); - /* The mech could probably be determined from the token instead, but it's just * as easy for now to pass it in. */ int -- cgit v1.2.2 From 75bc8821bd9ae76f2ffb92a46f7abdbb0aaa0536 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Sat, 16 Mar 2013 15:55:36 -0400 Subject: NFS: Handle missing rpc.gssd when looking up root FH When rpc.gssd is not running, any NFS operation that needs to use a GSS security flavor of course does not work. If looking up a server's root file handle results in an NFS4ERR_WRONGSEC, nfs4_find_root_sec() is called to try a bunch of security flavors until one works or all reasonable flavors have been tried. When rpc.gssd isn't running, this loop seems to fail immediately after rpcauth_create() craps out on the first GSS flavor. When the rpcauth_create() call in nfs4_lookup_root_sec() fails because rpc.gssd is not available, nfs4_lookup_root_sec() unconditionally returns -EIO. This prevents nfs4_find_root_sec() from retrying any other flavors; it drops out of its loop and fails immediately. Having nfs4_lookup_root_sec() return -EACCES instead allows nfs4_find_root_sec() to try all flavors in its list. Signed-off-by: Chuck Lever Cc: Bryan Schumaker Signed-off-by: Trond Myklebust --- fs/nfs/nfs4proc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index 81343944e096..f462dd884385 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -2506,7 +2506,7 @@ static int nfs4_lookup_root_sec(struct nfs_server *server, struct nfs_fh *fhandl auth = rpcauth_create(flavor, server->client); if (IS_ERR(auth)) { - ret = -EIO; + ret = -EACCES; goto out; } ret = nfs4_lookup_root(server, fhandle, info); -- cgit v1.2.2 From 2ed4b95b7ee5e5bcf77576ce5df4d7036584cd3c Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Sat, 16 Mar 2013 15:55:45 -0400 Subject: NFS: Clean up nfs4_proc_get_rootfh The long lines with no vertical white space make this function difficult for humans to read. Add a proper documenting comment while we're here. Signed-off-by: Chuck Lever Cc: Bryan Schumaker Signed-off-by: Trond Myklebust --- fs/nfs/nfs4proc.c | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index f462dd884385..173c988177e3 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -2547,24 +2547,36 @@ static int nfs4_find_root_sec(struct nfs_server *server, struct nfs_fh *fhandle, return status; } -/* - * get the file handle for the "/" directory on the server +static int nfs4_do_find_root_sec(struct nfs_server *server, + struct nfs_fh *fhandle, struct nfs_fsinfo *info) +{ + int mv = server->nfs_client->cl_minorversion; + return nfs_v4_minor_ops[mv]->find_root_sec(server, fhandle, info); +} + +/** + * nfs4_proc_get_rootfh - get file handle for server's pseudoroot + * @server: initialized nfs_server handle + * @fhandle: we fill in the pseudo-fs root file handle + * @info: we fill in an FSINFO struct + * + * Returns zero on success, or a negative errno. */ int nfs4_proc_get_rootfh(struct nfs_server *server, struct nfs_fh *fhandle, struct nfs_fsinfo *info) { - int minor_version = server->nfs_client->cl_minorversion; - int status = nfs4_lookup_root(server, fhandle, info); - if ((status == -NFS4ERR_WRONGSEC) && !(server->flags & NFS_MOUNT_SECFLAVOUR)) - /* - * A status of -NFS4ERR_WRONGSEC will be mapped to -EPERM - * by nfs4_map_errors() as this function exits. - */ - status = nfs_v4_minor_ops[minor_version]->find_root_sec(server, fhandle, info); + int status; + + status = nfs4_lookup_root(server, fhandle, info); + if ((status == -NFS4ERR_WRONGSEC) && + !(server->flags & NFS_MOUNT_SECFLAVOUR)) + status = nfs4_do_find_root_sec(server, fhandle, info); + if (status == 0) status = nfs4_server_capabilities(server, fhandle); if (status == 0) status = nfs4_do_fsinfo(server, fhandle, info); + return nfs4_map_errors(status); } -- cgit v1.2.2 From 83ca7f5ab31f57506bb35482e4b0426be653196a Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Sat, 16 Mar 2013 15:55:53 -0400 Subject: NFS: Avoid PUTROOTFH when managing leases Currently, the compound operation the Linux NFS client sends to the server to confirm a client ID looks like this: { SETCLIENTID_CONFIRM; PUTROOTFH; GETATTR(lease_time) } Once the lease is confirmed, it makes sense to know how long before the client will have to renew it. And, performing these operations in the same compound saves a round trip. Unfortunately, this arrangement assumes that the security flavor used for establishing a client ID can also be used to access the server's pseudo-fs. If the server requires a different security flavor to access its pseudo-fs than it allowed for the client's SETCLIENTID operation, the PUTROOTFH in this compound fails with NFS4ERR_WRONGSEC. Even though the SETCLIENTID_CONFIRM succeeded, our client's trunking detection logic interprets the failure of the compound as a failure by the server to confirm the client ID. As part of server trunking detection, the client then begins another SETCLIENTID pass with the same nfs4_client_id. This fails with NFS4ERR_CLID_INUSE because the first SETCLIENTID/SETCLIENTID_CONFIRM already succeeded in confirming that client ID -- it was the PUTROOTFH operation that caused the SETCLIENTID_CONFIRM compound to fail. To address this issue, separate the "establish client ID" step from the "accessing the server's pseudo-fs root" step. The first access of the server's pseudo-fs may require retrying the PUTROOTFH operation with different security flavors. This access is done in nfs4_proc_get_rootfh(). That leaves the matter of how to retrieve the server's lease time. nfs4_proc_fsinfo() already retrieves the lease time value, though none of its callers do anything with the retrieved value (nor do they mark the lease as "renewed"). Note that NFSv4.1 state recovery invokes nfs4_proc_get_lease_time() using the lease management security flavor. This may cause some heartburn if that security flavor isn't the same as the security flavor the server requires for accessing the pseudo-fs. Signed-off-by: Chuck Lever Cc: Bryan Schumaker Signed-off-by: Trond Myklebust --- fs/nfs/nfs4proc.c | 25 ++++++++++++------------- fs/nfs/nfs4xdr.c | 18 +++--------------- 2 files changed, 15 insertions(+), 28 deletions(-) diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index 173c988177e3..510a7dd84c46 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -3455,12 +3455,21 @@ static int _nfs4_do_fsinfo(struct nfs_server *server, struct nfs_fh *fhandle, static int nfs4_do_fsinfo(struct nfs_server *server, struct nfs_fh *fhandle, struct nfs_fsinfo *fsinfo) { struct nfs4_exception exception = { }; + unsigned long now = jiffies; int err; do { - err = nfs4_handle_exception(server, - _nfs4_do_fsinfo(server, fhandle, fsinfo), - &exception); + err = _nfs4_do_fsinfo(server, fhandle, fsinfo); + if (err == 0) { + struct nfs_client *clp = server->nfs_client; + + spin_lock(&clp->cl_lock); + clp->cl_lease_time = fsinfo->lease_time * HZ; + clp->cl_last_renewal = now; + spin_unlock(&clp->cl_lock); + break; + } + err = nfs4_handle_exception(server, err, &exception); } while (exception.retry); return err; } @@ -4301,27 +4310,17 @@ int nfs4_proc_setclientid_confirm(struct nfs_client *clp, struct nfs4_setclientid_res *arg, struct rpc_cred *cred) { - struct nfs_fsinfo fsinfo; struct rpc_message msg = { .rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_SETCLIENTID_CONFIRM], .rpc_argp = arg, - .rpc_resp = &fsinfo, .rpc_cred = cred, }; - unsigned long now; int status; dprintk("NFS call setclientid_confirm auth=%s, (client ID %llx)\n", clp->cl_rpcclient->cl_auth->au_ops->au_name, clp->cl_clientid); - now = jiffies; status = rpc_call_sync(clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT); - if (status == 0) { - spin_lock(&clp->cl_lock); - clp->cl_lease_time = fsinfo.lease_time * HZ; - clp->cl_last_renewal = now; - spin_unlock(&clp->cl_lock); - } dprintk("NFS reply setclientid_confirm: %d\n", status); return status; } diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c index a38fd179c34f..ae0190b82694 100644 --- a/fs/nfs/nfs4xdr.c +++ b/fs/nfs/nfs4xdr.c @@ -530,14 +530,10 @@ static int nfs4_stat_to_errno(int); decode_setclientid_maxsz) #define NFS4_enc_setclientid_confirm_sz \ (compound_encode_hdr_maxsz + \ - encode_setclientid_confirm_maxsz + \ - encode_putrootfh_maxsz + \ - encode_fsinfo_maxsz) + encode_setclientid_confirm_maxsz) #define NFS4_dec_setclientid_confirm_sz \ (compound_decode_hdr_maxsz + \ - decode_setclientid_confirm_maxsz + \ - decode_putrootfh_maxsz + \ - decode_fsinfo_maxsz) + decode_setclientid_confirm_maxsz) #define NFS4_enc_lock_sz (compound_encode_hdr_maxsz + \ encode_sequence_maxsz + \ encode_putfh_maxsz + \ @@ -2608,12 +2604,9 @@ static void nfs4_xdr_enc_setclientid_confirm(struct rpc_rqst *req, struct compound_hdr hdr = { .nops = 0, }; - const u32 lease_bitmap[3] = { FATTR4_WORD0_LEASE_TIME }; encode_compound_hdr(xdr, req, &hdr); encode_setclientid_confirm(xdr, arg, &hdr); - encode_putrootfh(xdr, &hdr); - encode_fsinfo(xdr, lease_bitmap, &hdr); encode_nops(&hdr); } @@ -6647,8 +6640,7 @@ static int nfs4_xdr_dec_setclientid(struct rpc_rqst *req, * Decode SETCLIENTID_CONFIRM response */ static int nfs4_xdr_dec_setclientid_confirm(struct rpc_rqst *req, - struct xdr_stream *xdr, - struct nfs_fsinfo *fsinfo) + struct xdr_stream *xdr) { struct compound_hdr hdr; int status; @@ -6656,10 +6648,6 @@ static int nfs4_xdr_dec_setclientid_confirm(struct rpc_rqst *req, status = decode_compound_hdr(xdr, &hdr); if (!status) status = decode_setclientid_confirm(xdr); - if (!status) - status = decode_putrootfh(xdr); - if (!status) - status = decode_fsinfo(xdr, fsinfo); return status; } -- cgit v1.2.2 From 9a744ba3983698307a7690541c00549ac7edf316 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Sat, 16 Mar 2013 15:56:02 -0400 Subject: NFS: Use static list of security flavors during root FH lookup recovery If the Linux NFS client receives an NFS4ERR_WRONGSEC error while trying to look up an NFS server's root file handle, it retries the lookup operation with various security flavors to see what flavor the NFS server will accept for pseudo-fs access. The list of flavors the client uses during retry consists only of flavors that are currently registered in the kernel RPC client. This list may not include any GSS pseudoflavors if auth_rpcgss.ko has not yet been loaded. Let's instead use a static list of security flavors that the NFS standard requires the server to implement (RFC 3530bis, section 3.2.1). The RPC client should now be able to load support for these dynamically; if not, they are skipped. Recovery behavior here is prescribed by RFC 3530bis, section 15.33.5: > For LOOKUPP, PUTROOTFH and PUTPUBFH, the client will be unable to > use the SECINFO operation since SECINFO requires a current > filehandle and none exist for these two [sic] operations. Therefore, > the client must iterate through the security triples available at > the client and reattempt the PUTROOTFH or PUTPUBFH operation. In > the unfortunate event none of the MANDATORY security triples are > supported by the client and server, the client SHOULD try using > others that support integrity. Failing that, the client can try > using AUTH_NONE, but because such forms lack integrity checks, > this puts the client at risk. Signed-off-by: Chuck Lever Cc: Bryan Schumaker Signed-off-by: Trond Myklebust --- fs/nfs/nfs4proc.c | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index 510a7dd84c46..dd3a43792df0 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -2514,27 +2514,35 @@ out: return ret; } +/* + * Retry pseudoroot lookup with various security flavors. We do this when: + * + * NFSv4.0: the PUTROOTFH operation returns NFS4ERR_WRONGSEC + * NFSv4.1: the server does not support the SECINFO_NO_NAME operation + * + * Returns zero on success, or a negative NFS4ERR value, or a + * negative errno value. + */ static int nfs4_find_root_sec(struct nfs_server *server, struct nfs_fh *fhandle, struct nfs_fsinfo *info) { - int i, len, status = 0; - rpc_authflavor_t flav_array[NFS_MAX_SECFLAVORS]; - - len = rpcauth_list_flavors(flav_array, ARRAY_SIZE(flav_array)); - if (len < 0) - return len; - - for (i = 0; i < len; i++) { - /* AUTH_UNIX is the default flavor if none was specified, - * thus has already been tried. */ - if (flav_array[i] == RPC_AUTH_UNIX) - continue; + /* Per 3530bis 15.33.5 */ + static const rpc_authflavor_t flav_array[] = { + RPC_AUTH_GSS_KRB5P, + RPC_AUTH_GSS_KRB5I, + RPC_AUTH_GSS_KRB5, + RPC_AUTH_NULL, + }; + int status = -EPERM; + size_t i; + for (i = 0; i < ARRAY_SIZE(flav_array); i++) { status = nfs4_lookup_root_sec(server, fhandle, info, flav_array[i]); if (status == -NFS4ERR_WRONGSEC || status == -EACCES) continue; break; } + /* * -EACCESS could mean that the user doesn't have correct permissions * to access the mount. It could also mean that we tried to mount -- cgit v1.2.2 From c4eafe1135809c2b35b873a395af8f3a86a3ee98 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Sat, 16 Mar 2013 15:56:11 -0400 Subject: NFS: Try AUTH_UNIX when PUTROOTFH gets NFS4ERR_WRONGSEC Most NFSv4 servers implement AUTH_UNIX, and administrators will prefer this over AUTH_NULL. It is harmless for our client to try this flavor in addition to the flavors mandated by RFC 3530/5661. Signed-off-by: Chuck Lever Signed-off-by: Trond Myklebust --- fs/nfs/nfs4proc.c | 1 + 1 file changed, 1 insertion(+) diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index dd3a43792df0..a8e6d802383b 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -2531,6 +2531,7 @@ static int nfs4_find_root_sec(struct nfs_server *server, struct nfs_fh *fhandle, RPC_AUTH_GSS_KRB5P, RPC_AUTH_GSS_KRB5I, RPC_AUTH_GSS_KRB5, + RPC_AUTH_UNIX, /* courtesy */ RPC_AUTH_NULL, }; int status = -EPERM; -- cgit v1.2.2 From 4edaa308888b4bd629fa025cc6d5b2bf1a2a51db Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Sat, 16 Mar 2013 15:56:20 -0400 Subject: NFS: Use "krb5i" to establish NFSv4 state whenever possible Currently our client uses AUTH_UNIX for state management on Kerberos NFS mounts in some cases. For example, if the first mount of a server specifies "sec=sys," the SETCLIENTID operation is performed with AUTH_UNIX. Subsequent mounts using stronger security flavors can not change the flavor used for lease establishment. This might be less security than an administrator was expecting. Dave Noveck's migration issues draft recommends the use of an integrity-protecting security flavor for the SETCLIENTID operation. Let's ignore the mount's sec= setting and use krb5i as the default security flavor for SETCLIENTID. If our client can't establish a GSS context (eg. because it doesn't have a keytab or the server doesn't support Kerberos) we fall back to using AUTH_NULL. For an operation that requires a machine credential (which never represents a particular user) AUTH_NULL is as secure as AUTH_UNIX. Signed-off-by: Chuck Lever Signed-off-by: Trond Myklebust --- fs/nfs/nfs4client.c | 2 +- fs/nfs/nfs4state.c | 37 +++++-------------------------------- 2 files changed, 6 insertions(+), 33 deletions(-) diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c index 17b34b2da2df..bb9789ef1d49 100644 --- a/fs/nfs/nfs4client.c +++ b/fs/nfs/nfs4client.c @@ -199,7 +199,7 @@ struct nfs_client *nfs4_init_client(struct nfs_client *clp, clp->rpc_ops = &nfs_v4_clientops; __set_bit(NFS_CS_DISCRTRY, &clp->cl_flags); - error = nfs_create_rpc_client(clp, timeparms, authflavour); + error = nfs_create_rpc_client(clp, timeparms, RPC_AUTH_GSS_KRB5I); if (error < 0) goto error; diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c index b924bdd69494..a30f51eb048f 100644 --- a/fs/nfs/nfs4state.c +++ b/fs/nfs/nfs4state.c @@ -1866,26 +1866,13 @@ int nfs4_discover_server_trunking(struct nfs_client *clp, { const struct nfs4_state_recovery_ops *ops = clp->cl_mvops->reboot_recovery_ops; - rpc_authflavor_t *flavors, flav, save; struct rpc_clnt *clnt; struct rpc_cred *cred; - int i, len, status; + int i, status; dprintk("NFS: %s: testing '%s'\n", __func__, clp->cl_hostname); - len = NFS_MAX_SECFLAVORS; - flavors = kcalloc(len, sizeof(*flavors), GFP_KERNEL); - if (flavors == NULL) { - status = -ENOMEM; - goto out; - } - len = rpcauth_list_flavors(flavors, len); - if (len < 0) { - status = len; - goto out_free; - } clnt = clp->cl_rpcclient; - save = clnt->cl_auth->au_flavor; i = 0; mutex_lock(&nfs_clid_init_mutex); @@ -1900,12 +1887,6 @@ again: switch (status) { case 0: break; - - case -EACCES: - if (clp->cl_machine_cred == NULL) - break; - /* Handle case where the user hasn't set up machine creds */ - nfs4_clear_machine_cred(clp); case -NFS4ERR_DELAY: case -ETIMEDOUT: case -EAGAIN: @@ -1914,17 +1895,12 @@ again: dprintk("NFS: %s after status %d, retrying\n", __func__, status); goto again; - + case -EACCES: + if (i++) + break; case -NFS4ERR_CLID_INUSE: case -NFS4ERR_WRONGSEC: - status = -EPERM; - if (i >= len) - break; - - flav = flavors[i++]; - if (flav == save) - flav = flavors[i++]; - clnt = rpc_clone_client_set_auth(clnt, flav); + clnt = rpc_clone_client_set_auth(clnt, RPC_AUTH_NULL); if (IS_ERR(clnt)) { status = PTR_ERR(clnt); break; @@ -1944,9 +1920,6 @@ again: out_unlock: mutex_unlock(&nfs_clid_init_mutex); -out_free: - kfree(flavors); -out: dprintk("NFS: %s: status = %d\n", __func__, status); return status; } -- cgit v1.2.2 From 1c74a244fcb61e6e1983d5725b8ccd5d3f51889c Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Fri, 22 Mar 2013 12:53:08 -0400 Subject: SUNRPC: Don't recognize RPC_AUTH_MAXFLAVOR RPC_AUTH_MAXFLAVOR is an invalid flavor, on purpose. Don't allow any processing whatsoever if a caller passes it to rpcauth_create() or rpcauth_get_gssinfo(). Signed-off-by: Chuck Lever Signed-off-by: Trond Myklebust --- net/sunrpc/auth.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/net/sunrpc/auth.c b/net/sunrpc/auth.c index 2bc0cc2196e0..ed2fdd210c0b 100644 --- a/net/sunrpc/auth.c +++ b/net/sunrpc/auth.c @@ -82,7 +82,7 @@ MODULE_PARM_DESC(auth_hashtable_size, "RPC credential cache hashtable size"); static u32 pseudoflavor_to_flavor(u32 flavor) { - if (flavor >= RPC_AUTH_MAXFLAVOR) + if (flavor > RPC_AUTH_MAXFLAVOR) return RPC_AUTH_GSS; return flavor; } @@ -173,6 +173,9 @@ rpcauth_get_gssinfo(rpc_authflavor_t pseudoflavor, struct rpcsec_gss_info *info) const struct rpc_authops *ops; int result; + if (flavor >= RPC_AUTH_MAXFLAVOR) + return -EINVAL; + ops = auth_flavors[flavor]; if (ops == NULL) request_module("rpc-auth-%u", flavor); -- cgit v1.2.2 From 4580a92d44e2b21c2254fa5fef0f1bfb43c82318 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Fri, 22 Mar 2013 12:53:17 -0400 Subject: NFS: Use server-recommended security flavor by default (NFSv3) Since commit ec88f28d in 2009, checking if the user-specified flavor is in the server's flavor list has been the source of a few noticeable regressions (now fixed), but there is one that is still vexing. An NFS server can list AUTH_NULL in its flavor list, which suggests a client should try to mount the server with the flavor of the client's choice, but the server will squash all accesses. In some cases, our client fails to mount a server because of this check, when the mount could have proceeded successfully. Skip this check if the user has specified "sec=" on the mount command line. But do consult the server-provided flavor list to choose a security flavor if no sec= option is specified on the mount command. If a server lists Kerberos pseudoflavors before "sys" in its export options, our client now chooses Kerberos over AUTH_UNIX for mount points, when no security flavor is specified by the mount command. This could be surprising to some administrators or users, who would then need to have Kerberos credentials to access the export. Or, a client administrator may not have enabled rpc.gssd. In this case, auth_rpcgss.ko might still be loadable, which is enough for the new logic to choose Kerberos over AUTH_UNIX. But the mount would fail since no GSS context can be created without rpc.gssd running. To retain the use of AUTH_UNIX by default: o The server administrator can ensure that "sys" is listed before Kerberos flavors in its export security options (see exports(5)), o The client administrator can explicitly specify "sec=sys" on its mount command line (see nfs(5)), o The client administrator can use "Sec=sys" in an appropriate section of /etc/nfsmount.conf (see nfsmount.conf(5)), or o The client administrator can blacklist auth_rpcgss.ko. Signed-off-by: Chuck Lever Signed-off-by: Trond Myklebust --- fs/nfs/nfs4super.c | 2 ++ fs/nfs/super.c | 80 ++++++++++++++++++++++++++++-------------------------- 2 files changed, 44 insertions(+), 38 deletions(-) diff --git a/fs/nfs/nfs4super.c b/fs/nfs/nfs4super.c index 569b166cc050..a5e1a3026d48 100644 --- a/fs/nfs/nfs4super.c +++ b/fs/nfs/nfs4super.c @@ -252,6 +252,8 @@ struct dentry *nfs4_try_mount(int flags, const char *dev_name, dfprintk(MOUNT, "--> nfs4_try_mount()\n"); + if (data->auth_flavors[0] == RPC_AUTH_MAXFLAVOR) + data->auth_flavors[0] = RPC_AUTH_UNIX; export_path = data->nfs_server.export_path; data->nfs_server.export_path = "/"; root_mnt = nfs_do_root_mount(&nfs4_remote_fs_type, flags, mount_info, diff --git a/fs/nfs/super.c b/fs/nfs/super.c index 17b32b722457..3bb8318f6d0c 100644 --- a/fs/nfs/super.c +++ b/fs/nfs/super.c @@ -917,7 +917,7 @@ static struct nfs_parsed_mount_data *nfs_alloc_parsed_mount_data(void) data->mount_server.port = NFS_UNSPEC_PORT; data->nfs_server.port = NFS_UNSPEC_PORT; data->nfs_server.protocol = XPRT_TRANSPORT_TCP; - data->auth_flavors[0] = RPC_AUTH_UNIX; + data->auth_flavors[0] = RPC_AUTH_MAXFLAVOR; data->auth_flavor_len = 1; data->minorversion = 0; data->need_mount = true; @@ -1605,49 +1605,57 @@ out_security_failure: } /* - * Match the requested auth flavors with the list returned by - * the server. Returns zero and sets the mount's authentication - * flavor on success; returns -EACCES if server does not support - * the requested flavor. + * Select a security flavor for this mount. The selected flavor + * is planted in args->auth_flavors[0]. */ -static int nfs_walk_authlist(struct nfs_parsed_mount_data *args, - struct nfs_mount_request *request) +static void nfs_select_flavor(struct nfs_parsed_mount_data *args, + struct nfs_mount_request *request) { - unsigned int i, j, server_authlist_len = *(request->auth_flav_len); + unsigned int i, count = *(request->auth_flav_len); + rpc_authflavor_t flavor; + + if (args->auth_flavors[0] != RPC_AUTH_MAXFLAVOR) + goto out; + + /* + * The NFSv2 MNT operation does not return a flavor list. + */ + if (args->mount_server.version != NFS_MNT3_VERSION) + goto out_default; /* * Certain releases of Linux's mountd return an empty - * flavor list. To prevent behavioral regression with - * these servers (ie. rejecting mounts that used to - * succeed), revert to pre-2.6.32 behavior (no checking) - * if the returned flavor list is empty. + * flavor list in some cases. */ - if (server_authlist_len == 0) - return 0; + if (count == 0) + goto out_default; /* - * We avoid sophisticated negotiating here, as there are - * plenty of cases where we can get it wrong, providing - * either too little or too much security. - * * RFC 2623, section 2.7 suggests we SHOULD prefer the * flavor listed first. However, some servers list - * AUTH_NULL first. Our caller plants AUTH_SYS, the - * preferred default, in args->auth_flavors[0] if user - * didn't specify sec= mount option. + * AUTH_NULL first. Avoid ever choosing AUTH_NULL. */ - for (i = 0; i < args->auth_flavor_len; i++) - for (j = 0; j < server_authlist_len; j++) - if (args->auth_flavors[i] == request->auth_flavs[j]) { - dfprintk(MOUNT, "NFS: using auth flavor %d\n", - request->auth_flavs[j]); - args->auth_flavors[0] = request->auth_flavs[j]; - return 0; - } + for (i = 0; i < count; i++) { + struct rpcsec_gss_info info; + + flavor = request->auth_flavs[i]; + switch (flavor) { + case RPC_AUTH_UNIX: + goto out_set; + case RPC_AUTH_NULL: + continue; + default: + if (rpcauth_get_gssinfo(flavor, &info) == 0) + goto out_set; + } + } - dfprintk(MOUNT, "NFS: server does not support requested auth flavor\n"); - nfs_umount(request); - return -EACCES; +out_default: + flavor = RPC_AUTH_UNIX; +out_set: + args->auth_flavors[0] = flavor; +out: + dfprintk(MOUNT, "NFS: using auth flavor %d\n", args->auth_flavors[0]); } /* @@ -1710,12 +1718,8 @@ static int nfs_request_mount(struct nfs_parsed_mount_data *args, return status; } - /* - * MNTv1 (NFSv2) does not support auth flavor negotiation. - */ - if (args->mount_server.version != NFS_MNT3_VERSION) - return 0; - return nfs_walk_authlist(args, &request); + nfs_select_flavor(args, &request); + return 0; } struct dentry *nfs_try_mount(int flags, const char *dev_name, -- cgit v1.2.2 From 23631227a672de5e81e167c3178b39e9a50f6541 Mon Sep 17 00:00:00 2001 From: Trond Myklebust Date: Thu, 4 Apr 2013 16:14:11 -0400 Subject: NFSv4: Fix the fallback to AUTH_NULL if krb5i is not available If the rpcsec_gss_krb5 module cannot be loaded, the attempt to create an rpc_client in nfs4_init_client will currently fail with an EINVAL. Fix is to retry with AUTH_NULL. Regression introduced by the commit "NFS: Use "krb5i" to establish NFSv4 state whenever possible" Signed-off-by: Trond Myklebust Cc: Chuck Lever Cc: Bryan Schumaker --- fs/nfs/nfs4client.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c index bb9789ef1d49..a4f2100fd9c2 100644 --- a/fs/nfs/nfs4client.c +++ b/fs/nfs/nfs4client.c @@ -200,6 +200,8 @@ struct nfs_client *nfs4_init_client(struct nfs_client *clp, __set_bit(NFS_CS_DISCRTRY, &clp->cl_flags); error = nfs_create_rpc_client(clp, timeparms, RPC_AUTH_GSS_KRB5I); + if (error == -EINVAL) + error = nfs_create_rpc_client(clp, timeparms, RPC_AUTH_NULL); if (error < 0) goto error; -- cgit v1.2.2 From ea33e6c3e79d23ef693b7ed3171ef90dddfbfc15 Mon Sep 17 00:00:00 2001 From: Trond Myklebust Date: Fri, 5 Apr 2013 13:22:50 -0400 Subject: NFSv4: Fix issues in nfs4_discover_server_trunking - Ensure that we exit with ENOENT if the call to ops->get_clid_cred() fails. - Handle the case where ops->detect_trunking() exits with an unexpected error, and return EIO. Signed-off-by: Trond Myklebust --- fs/nfs/nfs4state.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c index a30f51eb048f..209df6976a5b 100644 --- a/fs/nfs/nfs4state.c +++ b/fs/nfs/nfs4state.c @@ -1876,8 +1876,8 @@ int nfs4_discover_server_trunking(struct nfs_client *clp, i = 0; mutex_lock(&nfs_clid_init_mutex); - status = -ENOENT; again: + status = -ENOENT; cred = ops->get_clid_cred(clp); if (cred == NULL) goto out_unlock; @@ -1916,6 +1916,11 @@ again: case -NFS4ERR_NOT_SAME: /* FixMe: implement recovery * in nfs4_exchange_id */ status = -EKEYEXPIRED; + break; + default: + pr_warn("NFS: %s unhandled error %d. Exiting with error EIO\n", + __func__, status); + status = -EIO; } out_unlock: -- cgit v1.2.2 From 845cbceb22c67030df76552892ad4935669bf2e5 Mon Sep 17 00:00:00 2001 From: Trond Myklebust Date: Fri, 5 Apr 2013 15:37:04 -0400 Subject: NFSv4: Don't clear the machine cred when client establish returns EACCES The expected behaviour is that the client will decide at mount time whether or not to use a krb5i machine cred, or AUTH_NULL. Signed-off-by: Trond Myklebust Cc: Chuck Lever Cc: Bryan Schumaker --- fs/nfs/nfs4state.c | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c index 209df6976a5b..c54600962bc6 100644 --- a/fs/nfs/nfs4state.c +++ b/fs/nfs/nfs4state.c @@ -154,18 +154,6 @@ struct rpc_cred *nfs4_get_machine_cred_locked(struct nfs_client *clp) return cred; } -static void nfs4_clear_machine_cred(struct nfs_client *clp) -{ - struct rpc_cred *cred; - - spin_lock(&clp->cl_lock); - cred = clp->cl_machine_cred; - clp->cl_machine_cred = NULL; - spin_unlock(&clp->cl_lock); - if (cred != NULL) - put_rpccred(cred); -} - static struct rpc_cred * nfs4_get_renew_cred_server_locked(struct nfs_server *server) { @@ -1768,10 +1756,6 @@ static int nfs4_handle_reclaim_lease_error(struct nfs_client *clp, int status) clear_bit(NFS4CLNT_LEASE_CONFIRM, &clp->cl_state); return -EPERM; case -EACCES: - if (clp->cl_machine_cred == NULL) - return -EACCES; - /* Handle case where the user hasn't set up machine creds */ - nfs4_clear_machine_cred(clp); case -NFS4ERR_DELAY: case -ETIMEDOUT: case -EAGAIN: -- cgit v1.2.2 From a58e0be6f6b3eb2079b0b8fedc9df6fa86869f1e Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Fri, 22 Mar 2013 12:52:59 -0400 Subject: SUNRPC: Remove extra xprt_put() While testing error cases where rpc_new_client() fails, I saw some oopses. If rpc_new_client() fails, it already invokes xprt_put(). Thus __rpc_clone_client() does not need to invoke it again. Introduced by commit 1b63a751 "SUNRPC: Refactor rpc_clone_client()" Fri Sep 14, 2012. Signed-off-by: Chuck Lever Cc: stable@vger.kernel.org [>=3.7] Signed-off-by: Trond Myklebust --- net/sunrpc/clnt.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c index dcc446e7fbf6..9df26b785dc7 100644 --- a/net/sunrpc/clnt.c +++ b/net/sunrpc/clnt.c @@ -512,7 +512,7 @@ static struct rpc_clnt *__rpc_clone_client(struct rpc_create_args *args, new = rpc_new_client(args, xprt); if (IS_ERR(new)) { err = PTR_ERR(new); - goto out_put; + goto out_err; } atomic_inc(&clnt->cl_count); @@ -525,8 +525,6 @@ static struct rpc_clnt *__rpc_clone_client(struct rpc_create_args *args, new->cl_chatty = clnt->cl_chatty; return new; -out_put: - xprt_put(xprt); out_err: dprintk("RPC: %s: returned error %d\n", __func__, err); return ERR_PTR(err); -- cgit v1.2.2 From b193d59a4863ea670872d76dc99231ddeb598625 Mon Sep 17 00:00:00 2001 From: Trond Myklebust Date: Thu, 4 Apr 2013 15:55:00 -0400 Subject: NFSv4: Fix a memory leak in nfs4_discover_server_trunking When we assign a new rpc_client to clp->cl_rpcclient, we need to destroy the old one. Signed-off-by: Trond Myklebust Cc: Chuck Lever Cc: stable@vger.kernel.org [>=3.7] --- fs/nfs/nfs4state.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c index 6ace365c6334..d41a3518509f 100644 --- a/fs/nfs/nfs4state.c +++ b/fs/nfs/nfs4state.c @@ -1886,7 +1886,13 @@ again: status = PTR_ERR(clnt); break; } - clp->cl_rpcclient = clnt; + /* Note: this is safe because we haven't yet marked the + * client as ready, so we are the only user of + * clp->cl_rpcclient + */ + clnt = xchg(&clp->cl_rpcclient, clnt); + rpc_shutdown_client(clnt); + clnt = clp->cl_rpcclient; goto again; case -NFS4ERR_MINOR_VERS_MISMATCH: -- cgit v1.2.2 From 7b1f1fd1842e6ede25183c267ae733a7f67f00bc Mon Sep 17 00:00:00 2001 From: Trond Myklebust Date: Fri, 5 Apr 2013 16:11:11 -0400 Subject: NFSv4/4.1: Fix bugs in nfs4[01]_walk_client_list It is unsafe to use list_for_each_entry_safe() here, because when we drop the nn->nfs_client_lock, we pin the _current_ list entry and ensure that it stays in the list, but we don't do the same for the _next_ list entry. Use of list_for_each_entry() is therefore the correct thing to do. Also fix the refcounting in nfs41_walk_client_list(). Finally, ensure that the nfs_client has finished being initialised and, in the case of NFSv4.1, that the session is set up. Signed-off-by: Trond Myklebust Cc: Chuck Lever Cc: Bryan Schumaker Cc: stable@vger.kernel.org [>= 3.7] --- fs/nfs/nfs4client.c | 44 ++++++++++++++++++++++++++++---------------- 1 file changed, 28 insertions(+), 16 deletions(-) diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c index ac4fc9a8fdbc..c7b346f8cc44 100644 --- a/fs/nfs/nfs4client.c +++ b/fs/nfs/nfs4client.c @@ -300,7 +300,7 @@ int nfs40_walk_client_list(struct nfs_client *new, struct rpc_cred *cred) { struct nfs_net *nn = net_generic(new->cl_net, nfs_net_id); - struct nfs_client *pos, *n, *prev = NULL; + struct nfs_client *pos, *prev = NULL; struct nfs4_setclientid_res clid = { .clientid = new->cl_clientid, .confirm = new->cl_confirm, @@ -308,10 +308,23 @@ int nfs40_walk_client_list(struct nfs_client *new, int status = -NFS4ERR_STALE_CLIENTID; spin_lock(&nn->nfs_client_lock); - list_for_each_entry_safe(pos, n, &nn->nfs_client_list, cl_share_link) { + list_for_each_entry(pos, &nn->nfs_client_list, cl_share_link) { /* If "pos" isn't marked ready, we can't trust the * remaining fields in "pos" */ - if (pos->cl_cons_state < NFS_CS_READY) + if (pos->cl_cons_state > NFS_CS_READY) { + atomic_inc(&pos->cl_count); + spin_unlock(&nn->nfs_client_lock); + + if (prev) + nfs_put_client(prev); + prev = pos; + + status = nfs_wait_client_init_complete(pos); + spin_lock(&nn->nfs_client_lock); + if (status < 0) + continue; + } + if (pos->cl_cons_state != NFS_CS_READY) continue; if (pos->rpc_ops != new->rpc_ops) @@ -423,16 +436,16 @@ int nfs41_walk_client_list(struct nfs_client *new, struct rpc_cred *cred) { struct nfs_net *nn = net_generic(new->cl_net, nfs_net_id); - struct nfs_client *pos, *n, *prev = NULL; + struct nfs_client *pos, *prev = NULL; int status = -NFS4ERR_STALE_CLIENTID; spin_lock(&nn->nfs_client_lock); - list_for_each_entry_safe(pos, n, &nn->nfs_client_list, cl_share_link) { + list_for_each_entry(pos, &nn->nfs_client_list, cl_share_link) { /* If "pos" isn't marked ready, we can't trust the * remaining fields in "pos", especially the client * ID and serverowner fields. Wait for CREATE_SESSION * to finish. */ - if (pos->cl_cons_state < NFS_CS_READY) { + if (pos->cl_cons_state > NFS_CS_READY) { atomic_inc(&pos->cl_count); spin_unlock(&nn->nfs_client_lock); @@ -440,18 +453,17 @@ int nfs41_walk_client_list(struct nfs_client *new, nfs_put_client(prev); prev = pos; - nfs4_schedule_lease_recovery(pos); status = nfs_wait_client_init_complete(pos); - if (status < 0) { - nfs_put_client(pos); - spin_lock(&nn->nfs_client_lock); - continue; + if (status == 0) { + nfs4_schedule_lease_recovery(pos); + status = nfs4_wait_clnt_recover(pos); } - status = pos->cl_cons_state; spin_lock(&nn->nfs_client_lock); if (status < 0) continue; } + if (pos->cl_cons_state != NFS_CS_READY) + continue; if (pos->rpc_ops != new->rpc_ops) continue; @@ -469,17 +481,17 @@ int nfs41_walk_client_list(struct nfs_client *new, continue; atomic_inc(&pos->cl_count); - spin_unlock(&nn->nfs_client_lock); + *result = pos; dprintk("NFS: <-- %s using nfs_client = %p ({%d})\n", __func__, pos, atomic_read(&pos->cl_count)); - - *result = pos; - return 0; + break; } /* No matching nfs_client found. */ spin_unlock(&nn->nfs_client_lock); dprintk("NFS: <-- %s status = %d\n", __func__, status); + if (prev) + nfs_put_client(prev); return status; } #endif /* CONFIG_NFS_V4_1 */ -- cgit v1.2.2 From f05c124a70a4953a66acbd6d6c601ea1eb5d0fa7 Mon Sep 17 00:00:00 2001 From: Trond Myklebust Date: Fri, 5 Apr 2013 14:13:21 -0400 Subject: SUNRPC: Fix a potential memory leak in rpc_new_client If the call to rpciod_up() fails, we currently leak a reference to the struct rpc_xprt. As part of the fix, we also remove the redundant check for xprt!=NULL. This is already taken care of by the callers. Signed-off-by: Trond Myklebust --- net/sunrpc/clnt.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c index 9df26b785dc7..d5f35f15af98 100644 --- a/net/sunrpc/clnt.c +++ b/net/sunrpc/clnt.c @@ -304,10 +304,8 @@ static struct rpc_clnt * rpc_new_client(const struct rpc_create_args *args, stru err = rpciod_up(); if (err) goto out_no_rpciod; - err = -EINVAL; - if (!xprt) - goto out_no_xprt; + err = -EINVAL; if (args->version >= program->nrvers) goto out_err; version = program->version[args->version]; @@ -382,10 +380,9 @@ out_no_principal: out_no_stats: kfree(clnt); out_err: - xprt_put(xprt); -out_no_xprt: rpciod_down(); out_no_rpciod: + xprt_put(xprt); return ERR_PTR(err); } -- cgit v1.2.2 From fa332941c0c7c00e3420078268b7558d0ef792b5 Mon Sep 17 00:00:00 2001 From: Trond Myklebust Date: Tue, 9 Apr 2013 12:56:52 -0400 Subject: NFSv4: Fix another potential state manager deadlock Don't hold the NFSv4 sequence id while we check for open permission. The call to ACCESS may block due to reboot recovery. Signed-off-by: Trond Myklebust --- fs/nfs/nfs4proc.c | 1 + 1 file changed, 1 insertion(+) diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index 26431cf62ddb..0ad025eb523b 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -1046,6 +1046,7 @@ static struct nfs4_state *nfs4_try_open_cached(struct nfs4_opendata *opendata) /* Save the delegation */ nfs4_stateid_copy(&stateid, &delegation->stateid); rcu_read_unlock(); + nfs_release_seqid(opendata->o_arg.seqid); ret = nfs_may_open(state->inode, state->owner->so_cred, open_mode); if (ret != 0) goto out; -- cgit v1.2.2 From eb04e0ac198cec3bab407ad220438dfa65c19c67 Mon Sep 17 00:00:00 2001 From: Trond Myklebust Date: Wed, 10 Apr 2013 12:44:18 -0400 Subject: NFSv4: Doh! Typo in the fix to nfs41_walk_client_list Make sure that we set the status to 0 on success. Missed in testing because it never appears when doing multiple mounts to _different_ servers. Signed-off-by: Trond Myklebust Cc: # 3.7.x: 7b1f1fd: NFSv4/4.1: Fix bugs in nfs4[01]_walk_client_list --- fs/nfs/nfs4client.c | 1 + 1 file changed, 1 insertion(+) diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c index c7b346f8cc44..66b6664dcd4c 100644 --- a/fs/nfs/nfs4client.c +++ b/fs/nfs/nfs4client.c @@ -482,6 +482,7 @@ int nfs41_walk_client_list(struct nfs_client *new, atomic_inc(&pos->cl_count); *result = pos; + status = 0; dprintk("NFS: <-- %s using nfs_client = %p ({%d})\n", __func__, pos, atomic_read(&pos->cl_count)); break; -- cgit v1.2.2 From 79d852bf5e7691dc78cc6322ecd1860c50940785 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Mon, 22 Apr 2013 15:42:48 -0400 Subject: NFS: Retry SETCLIENTID with AUTH_SYS instead of AUTH_NONE Recently I changed the SETCLIENTID code to use AUTH_GSS(krb5i), and then retry with AUTH_NONE if that didn't work. This was to enable Kerberos NFS mounts to work without forcing Linux NFS clients to have a keytab on hand. Rick Macklem reports that the FreeBSD server accepts AUTH_NONE only for NULL operations (thus certainly not for SETCLIENTID). Falling back to AUTH_NONE means our proposed 3.10 NFS client will not interoperate with FreeBSD servers over NFSv4 unless Kerberos is fully configured on both ends. If the Linux client falls back to using AUTH_SYS instead for SETCLIENTID, all should work fine as long as the NFS server is configured to allow AUTH_SYS for SETCLIENTID. This may still prevent access to Kerberos-only FreeBSD servers by Linux clients with no keytab. Rick is of the opinion that the security settings the server applies to its pseudo-fs should also apply to the SETCLIENTID operation. Linux and Solaris NFS servers do not place that limitation on SETCLIENTID. The security settings for the server's pseudo-fs are determined automatically as the union of security flavors allowed on real exports, as recommended by RFC 3530bis; and the flavors allowed for SETCLIENTID are all flavors supported by the respective server implementation. Signed-off-by: Chuck Lever Signed-off-by: Trond Myklebust --- fs/nfs/nfs4state.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c index c54600962bc6..a75e712490f4 100644 --- a/fs/nfs/nfs4state.c +++ b/fs/nfs/nfs4state.c @@ -1884,7 +1884,7 @@ again: break; case -NFS4ERR_CLID_INUSE: case -NFS4ERR_WRONGSEC: - clnt = rpc_clone_client_set_auth(clnt, RPC_AUTH_NULL); + clnt = rpc_clone_client_set_auth(clnt, RPC_AUTH_UNIX); if (IS_ERR(clnt)) { status = PTR_ERR(clnt); break; -- cgit v1.2.2 From 8188df1733d6722336e287cc859b2567ad70576a Mon Sep 17 00:00:00 2001 From: Trond Myklebust Date: Tue, 23 Apr 2013 14:31:19 -0400 Subject: NFSv4.1: Use the more efficient open_noattr call for open-by-filehandle When we're doing open-by-filehandle in NFSv4.1, we shouldn't need to do the cache consistency revalidation on the directory. It is therefore more efficient to just use open_noattr, which returns the file attributes, but not the directory attributes. Signed-off-by: Trond Myklebust --- fs/nfs/nfs4proc.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index c13144911d20..a411a53d8f4a 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -1549,9 +1549,13 @@ static void nfs4_open_prepare(struct rpc_task *task, void *calldata) } /* Update client id. */ data->o_arg.clientid = clp->cl_clientid; - if (data->o_arg.claim == NFS4_OPEN_CLAIM_PREVIOUS) { - task->tk_msg.rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_OPEN_NOATTR]; + switch (data->o_arg.claim) { + case NFS4_OPEN_CLAIM_PREVIOUS: + case NFS4_OPEN_CLAIM_DELEG_CUR_FH: + case NFS4_OPEN_CLAIM_DELEG_PREV_FH: data->o_arg.open_bitmap = &nfs4_open_noattr_bitmap[0]; + case NFS4_OPEN_CLAIM_FH: + task->tk_msg.rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_OPEN_NOATTR]; nfs_copy_fh(&data->o_res.fh, data->o_arg.fh); } data->timestamp = jiffies; -- cgit v1.2.2 From cd4c9be2c61d4d974d348743fddb8183b8185abc Mon Sep 17 00:00:00 2001 From: Trond Myklebust Date: Tue, 23 Apr 2013 14:46:25 -0400 Subject: NFSv4.1: Don't do a delegated open for NFS4_OPEN_CLAIM_DELEG_CUR_FH modes If we're in a delegation recall situation, we can't do a delegated open. Signed-off-by: Trond Myklebust --- fs/nfs/nfs4proc.c | 1 + 1 file changed, 1 insertion(+) diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index a411a53d8f4a..ea50807b65d9 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -1543,6 +1543,7 @@ static void nfs4_open_prepare(struct rpc_task *task, void *calldata) rcu_read_lock(); delegation = rcu_dereference(NFS_I(data->state->inode)->delegation); if (data->o_arg.claim != NFS4_OPEN_CLAIM_DELEGATE_CUR && + data->o_arg.claim != NFS4_OPEN_CLAIM_DELEG_CUR_FH && can_open_delegated(delegation, data->o_arg.fmode)) goto unlock_no_action; rcu_read_unlock(); -- cgit v1.2.2 From bdeca1b76cd56cd10a029f0ad2fd9ab6dd7e313d Mon Sep 17 00:00:00 2001 From: Trond Myklebust Date: Tue, 23 Apr 2013 14:52:44 -0400 Subject: NFSv4: Don't recheck permissions on open in case of recovery cached open If we already checked the user access permissions on the original open, then don't bother checking again on recovery. Doing so can cause a deadlock with NFSv4.1, since the may_open() operation is not privileged. Furthermore, we can't report an access permission failure here anyway. Signed-off-by: Trond Myklebust --- fs/nfs/nfs4proc.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index ea50807b65d9..e18b3b46c001 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -769,6 +769,7 @@ struct nfs4_opendata { struct iattr attrs; unsigned long timestamp; unsigned int rpc_done : 1; + unsigned int is_recover : 1; int rpc_status; int cancelled; }; @@ -1101,9 +1102,11 @@ static struct nfs4_state *nfs4_try_open_cached(struct nfs4_opendata *opendata) /* Save the delegation */ nfs4_stateid_copy(&stateid, &delegation->stateid); rcu_read_unlock(); - ret = nfs_may_open(state->inode, state->owner->so_cred, open_mode); - if (ret != 0) - goto out; + if (!opendata->is_recover) { + ret = nfs_may_open(state->inode, state->owner->so_cred, open_mode); + if (ret != 0) + goto out; + } ret = -EAGAIN; /* Try to update the stateid using the delegation */ @@ -1670,8 +1673,11 @@ static int nfs4_run_open_task(struct nfs4_opendata *data, int isrecover) data->rpc_done = 0; data->rpc_status = 0; data->cancelled = 0; - if (isrecover) + data->is_recover = 0; + if (isrecover) { nfs4_set_sequence_privileged(&o_arg->seq_args); + data->is_recover = 1; + } task = rpc_run_task(&task_setup_data); if (IS_ERR(task)) return PTR_ERR(task); -- cgit v1.2.2 From ee3ae84ef40154c734dc2cbca5cf4b2f54c6c7c7 Mon Sep 17 00:00:00 2001 From: Trond Myklebust Date: Mon, 29 Apr 2013 10:35:36 -0400 Subject: NFSv4: Servers should only check SETATTR stateid open mode on size change The NFSv4 and NFSv4.1 specs are both clear that the server should only check stateid open mode if a SETATTR specifies the size attribute. If the open mode is not one that allows writing, then it returns NFS4ERR_OPENMODE. In the case where the SETATTR is not changing the size, the client will still pass it the delegation stateid to ensure that the server does not recall that delegation. In that case, the server should _ignore_ the delegation open mode, and simply apply standard permission checks. Signed-off-by: Trond Myklebust --- fs/nfs/nfs4proc.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index 3bc847ce4838..982b4527551d 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -2142,20 +2142,25 @@ static int _nfs4_do_setattr(struct inode *inode, struct rpc_cred *cred, .rpc_cred = cred, }; unsigned long timestamp = jiffies; + fmode_t fmode; + bool truncate; int status; nfs_fattr_init(fattr); - if (state != NULL && nfs4_valid_open_stateid(state)) { + /* Servers should only apply open mode checks for file size changes */ + truncate = (sattr->ia_valid & ATTR_SIZE) ? true : false; + fmode = truncate ? FMODE_WRITE : FMODE_READ; + + if (nfs4_copy_delegation_stateid(&arg.stateid, inode, fmode)) { + /* Use that stateid */ + } else if (truncate && state != NULL && nfs4_valid_open_stateid(state)) { struct nfs_lockowner lockowner = { .l_owner = current->files, .l_pid = current->tgid, }; nfs4_select_rw_stateid(&arg.stateid, state, FMODE_WRITE, &lockowner); - } else if (nfs4_copy_delegation_stateid(&arg.stateid, inode, - FMODE_WRITE)) { - /* Use that stateid */ } else nfs4_stateid_copy(&arg.stateid, &zero_stateid); -- cgit v1.2.2 From 721ccfb79b6f74f4052de70236d24047e73682d4 Mon Sep 17 00:00:00 2001 From: Trond Myklebust Date: Mon, 29 Apr 2013 11:11:58 -0400 Subject: NFSv4: Warn once about servers that incorrectly apply open mode to setattr Debugging aid to help identify servers that incorrectly apply open mode checks to setattr requests that are not changing the file size. Signed-off-by: Trond Myklebust --- fs/nfs/nfs4proc.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index 982b4527551d..9da4bd55eb30 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -2184,6 +2184,13 @@ static int nfs4_do_setattr(struct inode *inode, struct rpc_cred *cred, err = _nfs4_do_setattr(inode, cred, fattr, sattr, state); switch (err) { case -NFS4ERR_OPENMODE: + if (!(sattr->ia_valid & ATTR_SIZE)) { + pr_warn_once("NFSv4: server %s is incorrectly " + "applying open mode checks to " + "a SETATTR that is not " + "changing file size.\n", + server->nfs_client->cl_hostname); + } if (state && !(state->state & FMODE_WRITE)) { err = -EBADF; if (sattr->ia_valid & ATTR_OPEN) -- cgit v1.2.2