* [PATCH 0/3] Three short NFSD subjects
@ 2013-04-30 22:48 Chuck Lever
2013-04-30 22:48 ` [PATCH 1/3] NFSD: Simplify GSS flavor encoding in nfsd4_do_encode_secinfo() Chuck Lever
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Chuck Lever @ 2013-04-30 22:48 UTC (permalink / raw)
To: bfields; +Cc: linux-nfs
Hi Bruce-
Two clean up patches, and a fix for a SECINFO corner case.
---
Chuck Lever (3):
NFSD: Kill some mixed sign comparisons in fs/nfsd/nfs4xdr.c
NFSD: SECINFO doesn't handle unsupported pseudoflavors correctly
NFSD: Simplify GSS flavor encoding in nfsd4_do_encode_secinfo()
fs/nfsd/nfs4xdr.c | 43 +++++++++++++++++++++++++------------------
fs/nfsd/xdr4.h | 2 +-
2 files changed, 26 insertions(+), 19 deletions(-)
--
Chuck Lever
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/3] NFSD: Simplify GSS flavor encoding in nfsd4_do_encode_secinfo()
2013-04-30 22:48 [PATCH 0/3] Three short NFSD subjects Chuck Lever
@ 2013-04-30 22:48 ` Chuck Lever
2013-04-30 22:48 ` [PATCH 2/3] NFSD: SECINFO doesn't handle unsupported pseudoflavors correctly Chuck Lever
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Chuck Lever @ 2013-04-30 22:48 UTC (permalink / raw)
To: bfields; +Cc: linux-nfs
Clean up.
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
fs/nfsd/nfs4xdr.c | 8 +-------
1 files changed, 1 insertions(+), 7 deletions(-)
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 2502951..ce327e3 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -3174,17 +3174,11 @@ nfsd4_do_encode_secinfo(struct nfsd4_compoundres *resp,
struct rpcsec_gss_info info;
if (rpcauth_get_gssinfo(flavs[i].pseudoflavor, &info) == 0) {
- RESERVE_SPACE(4);
+ RESERVE_SPACE(4 + 4 + info.oid.len + 4 + 4);
WRITE32(RPC_AUTH_GSS);
- ADJUST_ARGS();
- RESERVE_SPACE(4 + info.oid.len);
WRITE32(info.oid.len);
WRITEMEM(info.oid.data, info.oid.len);
- ADJUST_ARGS();
- RESERVE_SPACE(4);
WRITE32(info.qop);
- ADJUST_ARGS();
- RESERVE_SPACE(4);
WRITE32(info.service);
ADJUST_ARGS();
} else {
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/3] NFSD: SECINFO doesn't handle unsupported pseudoflavors correctly
2013-04-30 22:48 [PATCH 0/3] Three short NFSD subjects Chuck Lever
2013-04-30 22:48 ` [PATCH 1/3] NFSD: Simplify GSS flavor encoding in nfsd4_do_encode_secinfo() Chuck Lever
@ 2013-04-30 22:48 ` Chuck Lever
2013-04-30 22:49 ` [PATCH 3/3] NFSD: Kill some mixed sign comparisons in fs/nfsd/nfs4xdr.c Chuck Lever
2013-04-30 23:42 ` [PATCH 0/3] Three short NFSD subjects J. Bruce Fields
3 siblings, 0 replies; 6+ messages in thread
From: Chuck Lever @ 2013-04-30 22:48 UTC (permalink / raw)
To: bfields; +Cc: linux-nfs
If nfsd4_do_encode_secinfo() can't find GSS info that matches an
export security flavor, it assumes the flavor is not a GSS
pseudoflavor, and simply puts it on the wire.
However, if this XDR encoding logic is given a legitimate GSS
pseudoflavor but the RPC layer says it does not support that
pseudoflavor for some reason, then the server leaks GSS pseudoflavor
numbers onto the wire.
I confirmed this happens by blacklisting rpcsec_gss_krb5, then
attempted a client transition from the pseudo-fs to a Kerberos-only
share. The client received a flavor list containing the Kerberos
pseudoflavor numbers, rather than GSS tuples.
The encoder logic can check that each pseudoflavor in flavs[] is
less than MAXFLAVOR before writing it into the buffer, to prevent
this. But after "nflavs" is written into the XDR buffer, the
encoder can't skip writing flavor information into the buffer when
it discovers the RPC layer doesn't support that flavor.
So count the number of valid flavors as they are written into the
XDR buffer, then write that count into a placeholder in the XDR
buffer when all recognized flavors have been encoded.
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
fs/nfsd/nfs4xdr.c | 26 ++++++++++++++++++++------
1 files changed, 20 insertions(+), 6 deletions(-)
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index ce327e3..758a1f3 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -3140,10 +3140,11 @@ static __be32
nfsd4_do_encode_secinfo(struct nfsd4_compoundres *resp,
__be32 nfserr, struct svc_export *exp)
{
- u32 i, nflavs;
+ u32 i, nflavs, supported;
struct exp_flavor_info *flavs;
struct exp_flavor_info def_flavs[2];
- __be32 *p;
+ __be32 *p, *flavorsp;
+ static bool report = true;
if (nfserr)
goto out;
@@ -3167,13 +3168,17 @@ nfsd4_do_encode_secinfo(struct nfsd4_compoundres *resp,
}
}
+ supported = 0;
RESERVE_SPACE(4);
- WRITE32(nflavs);
+ flavorsp = p++; /* to be backfilled later */
ADJUST_ARGS();
+
for (i = 0; i < nflavs; i++) {
+ rpc_authflavor_t pf = flavs[i].pseudoflavor;
struct rpcsec_gss_info info;
- if (rpcauth_get_gssinfo(flavs[i].pseudoflavor, &info) == 0) {
+ if (rpcauth_get_gssinfo(pf, &info) == 0) {
+ supported++;
RESERVE_SPACE(4 + 4 + info.oid.len + 4 + 4);
WRITE32(RPC_AUTH_GSS);
WRITE32(info.oid.len);
@@ -3181,13 +3186,22 @@ nfsd4_do_encode_secinfo(struct nfsd4_compoundres *resp,
WRITE32(info.qop);
WRITE32(info.service);
ADJUST_ARGS();
- } else {
+ } else if (pf < RPC_AUTH_MAXFLAVOR) {
+ supported++;
RESERVE_SPACE(4);
- WRITE32(flavs[i].pseudoflavor);
+ WRITE32(pf);
ADJUST_ARGS();
+ } else {
+ if (report)
+ pr_warn("NFS: SECINFO: security flavor %u "
+ "is not supported\n", pf);
}
}
+ if (nflavs != supported)
+ report = false;
+ *flavorsp = htonl(supported);
+
out:
if (exp)
exp_put(exp);
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/3] NFSD: Kill some mixed sign comparisons in fs/nfsd/nfs4xdr.c
2013-04-30 22:48 [PATCH 0/3] Three short NFSD subjects Chuck Lever
2013-04-30 22:48 ` [PATCH 1/3] NFSD: Simplify GSS flavor encoding in nfsd4_do_encode_secinfo() Chuck Lever
2013-04-30 22:48 ` [PATCH 2/3] NFSD: SECINFO doesn't handle unsupported pseudoflavors correctly Chuck Lever
@ 2013-04-30 22:49 ` Chuck Lever
2013-04-30 23:42 ` J. Bruce Fields
2013-04-30 23:42 ` [PATCH 0/3] Three short NFSD subjects J. Bruce Fields
3 siblings, 1 reply; 6+ messages in thread
From: Chuck Lever @ 2013-04-30 22:49 UTC (permalink / raw)
To: bfields; +Cc: linux-nfs
Clean up.
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
fs/nfsd/nfs4xdr.c | 9 ++++-----
fs/nfsd/xdr4.h | 2 +-
2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 758a1f3..39d60a0 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -244,7 +244,7 @@ static __be32
nfsd4_decode_fattr(struct nfsd4_compoundargs *argp, u32 *bmval,
struct iattr *iattr, struct nfs4_acl **acl)
{
- int expected_len, len = 0;
+ u32 expected_len, len = 0;
u32 dummy32;
char *buf;
int host_err;
@@ -1132,8 +1132,7 @@ nfsd4_decode_verify(struct nfsd4_compoundargs *argp, struct nfsd4_verify *verify
static __be32
nfsd4_decode_write(struct nfsd4_compoundargs *argp, struct nfsd4_write *write)
{
- int avail;
- int len;
+ u32 avail, len;
DECODE_HEAD;
status = nfsd4_decode_stateid(argp, &write->wr_stateid);
@@ -1200,7 +1199,7 @@ static __be32
nfsd4_decode_exchange_id(struct nfsd4_compoundargs *argp,
struct nfsd4_exchange_id *exid)
{
- int dummy, tmp;
+ u32 dummy, tmp;
DECODE_HEAD;
READ_BUF(NFS4_VERIFIER_SIZE);
@@ -3033,7 +3032,7 @@ nfsd4_encode_readlink(struct nfsd4_compoundres *resp, __be32 nfserr, struct nfsd
static __be32
nfsd4_encode_readdir(struct nfsd4_compoundres *resp, __be32 nfserr, struct nfsd4_readdir *readdir)
{
- int maxcount;
+ unsigned int maxcount;
loff_t offset;
__be32 *page, *savep, *tailbase;
__be32 *p;
diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
index 546f898..c84aea7 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -482,7 +482,7 @@ struct nfsd4_compoundargs {
__be32 * p;
__be32 * end;
struct page ** pagelist;
- int pagelen;
+ unsigned int pagelen;
__be32 tmp[8];
__be32 * tmpp;
struct tmpbuf {
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 3/3] NFSD: Kill some mixed sign comparisons in fs/nfsd/nfs4xdr.c
2013-04-30 22:49 ` [PATCH 3/3] NFSD: Kill some mixed sign comparisons in fs/nfsd/nfs4xdr.c Chuck Lever
@ 2013-04-30 23:42 ` J. Bruce Fields
0 siblings, 0 replies; 6+ messages in thread
From: J. Bruce Fields @ 2013-04-30 23:42 UTC (permalink / raw)
To: Chuck Lever; +Cc: linux-nfs
On Tue, Apr 30, 2013 at 06:49:03PM -0400, Chuck Lever wrote:
> Clean up.
I don't want to describe something as cleanup unless it's obvious it
won't change behavior. But:
> @@ -3033,7 +3032,7 @@ nfsd4_encode_readlink(struct nfsd4_compoundres *resp, __be32 nfserr, struct nfsd
> static __be32
> nfsd4_encode_readdir(struct nfsd4_compoundres *resp, __be32 nfserr, struct nfsd4_readdir *readdir)
> {
> - int maxcount;
> + unsigned int maxcount;
> loff_t offset;
> __be32 *page, *savep, *tailbase;
> __be32 *p;
This one, at least, is an actual behavior change. (Note we've got an
"if (maxcount < 0) ..." later on.) Maybe not a real problem, I haven't
checked.
And I haven't checked the others carefully.
--b.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 0/3] Three short NFSD subjects
2013-04-30 22:48 [PATCH 0/3] Three short NFSD subjects Chuck Lever
` (2 preceding siblings ...)
2013-04-30 22:49 ` [PATCH 3/3] NFSD: Kill some mixed sign comparisons in fs/nfsd/nfs4xdr.c Chuck Lever
@ 2013-04-30 23:42 ` J. Bruce Fields
3 siblings, 0 replies; 6+ messages in thread
From: J. Bruce Fields @ 2013-04-30 23:42 UTC (permalink / raw)
To: Chuck Lever; +Cc: linux-nfs
On Tue, Apr 30, 2013 at 06:48:37PM -0400, Chuck Lever wrote:
> Hi Bruce-
>
> Two clean up patches, and a fix for a SECINFO corner case.
The first two look good, thanks.--b.
>
> ---
>
> Chuck Lever (3):
> NFSD: Kill some mixed sign comparisons in fs/nfsd/nfs4xdr.c
> NFSD: SECINFO doesn't handle unsupported pseudoflavors correctly
> NFSD: Simplify GSS flavor encoding in nfsd4_do_encode_secinfo()
>
>
> fs/nfsd/nfs4xdr.c | 43 +++++++++++++++++++++++++------------------
> fs/nfsd/xdr4.h | 2 +-
> 2 files changed, 26 insertions(+), 19 deletions(-)
>
> --
> Chuck Lever
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-04-30 23:42 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-30 22:48 [PATCH 0/3] Three short NFSD subjects Chuck Lever
2013-04-30 22:48 ` [PATCH 1/3] NFSD: Simplify GSS flavor encoding in nfsd4_do_encode_secinfo() Chuck Lever
2013-04-30 22:48 ` [PATCH 2/3] NFSD: SECINFO doesn't handle unsupported pseudoflavors correctly Chuck Lever
2013-04-30 22:49 ` [PATCH 3/3] NFSD: Kill some mixed sign comparisons in fs/nfsd/nfs4xdr.c Chuck Lever
2013-04-30 23:42 ` J. Bruce Fields
2013-04-30 23:42 ` [PATCH 0/3] Three short NFSD subjects J. Bruce Fields
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).