2014-12-07 21:05:49

by Benjamin Coddington

[permalink] [raw]
Subject: [PATCH 0/2] Two nfsd referral xdr encoding bugs

Found a couple of xdr encoding problems in fs_location for referrals. These
should fix up:
https://bugzilla.redhat.com/show_bug.cgi?id=1164055
nfs referral mount fail with "mount(2): Input/output error"

Benjamin Coddington (2):
nfsd4: fix xdr4 inclusion of escaped char
nfsd4: fix xdr4 count of server in fs_location4

fs/nfsd/nfs4xdr.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)



2014-12-07 21:05:49

by Benjamin Coddington

[permalink] [raw]
Subject: [PATCH 1/2] nfsd4: fix xdr4 inclusion of escaped char

Fix a bug where nfsd4_encode_components_esc() includes the esc_end char as
an additional string encoding.

Signed-off-by: Benjamin Coddington <[email protected]>
---
fs/nfsd/nfs4xdr.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index eeea7a9..6c92a53 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -1795,6 +1795,9 @@ static __be32 nfsd4_encode_components_esc(struct xdr_stream *xdr, char sep,
}
else
end++;
+ if (found_esc)
+ end = next;
+
str = end;
}
pathlen = htonl(xdr->buf->len - pathlen_offset);
--
1.7.1


2014-12-09 19:57:19

by Benjamin Coddington

[permalink] [raw]
Subject: Re: [PATCH 1/2] nfsd4: fix xdr4 inclusion of escaped char

On Tue, 9 Dec 2014, J. Bruce Fields wrote:

> On Sun, Dec 07, 2014 at 04:05:47PM -0500, Benjamin Coddington wrote:
> > Fix a bug where nfsd4_encode_components_esc() includes the esc_end char as
> > an additional string encoding.
>
> Has it had this problem since the escaping was added by e7a0444aef4a
> "mfsd" add addr escaping to fs_location hosts"? I wonder why it wasn't
> noticed then. Maybe the client always just chooses the first entry and
> doesn't care if there's a second entry for a host named "]".
>
> --b.

I think it has been there all along. The linux client doesn't appear to
care about the second entry.

Ben

> >
> > Signed-off-by: Benjamin Coddington <[email protected]>
> > ---
> > fs/nfsd/nfs4xdr.c | 3 +++
> > 1 files changed, 3 insertions(+), 0 deletions(-)
> >
> > diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> > index eeea7a9..6c92a53 100644
> > --- a/fs/nfsd/nfs4xdr.c
> > +++ b/fs/nfsd/nfs4xdr.c
> > @@ -1795,6 +1795,9 @@ static __be32 nfsd4_encode_components_esc(struct xdr_stream *xdr, char sep,
> > }
> > else
> > end++;
> > + if (found_esc)
> > + end = next;
> > +
> > str = end;
> > }
> > pathlen = htonl(xdr->buf->len - pathlen_offset);
> > --
> > 1.7.1
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> > the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>

2014-12-07 21:05:49

by Benjamin Coddington

[permalink] [raw]
Subject: [PATCH 2/2] nfsd4: fix xdr4 count of server in fs_location4

Fix a bug where nfsd4_encode_components_esc() incorrectly calculates the
length of server array in fs_location4.

Signed-off-by: Benjamin Coddington <[email protected]>
---
fs/nfsd/nfs4xdr.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 6c92a53..2a77603 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -1800,7 +1800,7 @@ static __be32 nfsd4_encode_components_esc(struct xdr_stream *xdr, char sep,

str = end;
}
- pathlen = htonl(xdr->buf->len - pathlen_offset);
+ pathlen = htonl(count);
write_bytes_to_xdr_buf(xdr->buf, pathlen_offset, &pathlen, 4);
return 0;
}
--
1.7.1


2014-12-09 19:24:10

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 1/2] nfsd4: fix xdr4 inclusion of escaped char

On Sun, Dec 07, 2014 at 04:05:47PM -0500, Benjamin Coddington wrote:
> Fix a bug where nfsd4_encode_components_esc() includes the esc_end char as
> an additional string encoding.

Has it had this problem since the escaping was added by e7a0444aef4a
"mfsd" add addr escaping to fs_location hosts"? I wonder why it wasn't
noticed then. Maybe the client always just chooses the first entry and
doesn't care if there's a second entry for a host named "]".

--b.

>
> Signed-off-by: Benjamin Coddington <[email protected]>
> ---
> fs/nfsd/nfs4xdr.c | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> index eeea7a9..6c92a53 100644
> --- a/fs/nfsd/nfs4xdr.c
> +++ b/fs/nfsd/nfs4xdr.c
> @@ -1795,6 +1795,9 @@ static __be32 nfsd4_encode_components_esc(struct xdr_stream *xdr, char sep,
> }
> else
> end++;
> + if (found_esc)
> + end = next;
> +
> str = end;
> }
> pathlen = htonl(xdr->buf->len - pathlen_offset);
> --
> 1.7.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2014-12-09 17:37:11

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 2/2] nfsd4: fix xdr4 count of server in fs_location4

On Sun, Dec 07, 2014 at 04:05:48PM -0500, Benjamin Coddington wrote:
> Fix a bug where nfsd4_encode_components_esc() incorrectly calculates the
> length of server array in fs_location4.

Thanks, applying as follows.

This is something I don't test regularly but should.

--b.

commit 3e376626896b
Author: Benjamin Coddington <[email protected]>
Date: Sun Dec 7 16:05:48 2014 -0500

nfsd4: fix xdr4 count of server in fs_location4

Fix a bug where nfsd4_encode_components_esc() incorrectly calculates the
length of server array in fs_location4--note that it is a count of the
number of array elements, not a length in bytes.

Signed-off-by: Benjamin Coddington <[email protected]>
Fixes: 082d4bd72a45 (nfsd4: "backfill" using write_bytes_to_xdr_buf)
Cc: [email protected]
Signed-off-by: J. Bruce Fields <[email protected]>

diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index a8549f8fef57..e578c87d5527 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -1817,7 +1817,7 @@ static __be32 nfsd4_encode_components_esc(struct xdr_stream *xdr, char sep,

str = end;
}
- pathlen = htonl(xdr->buf->len - pathlen_offset);
+ pathlen = htonl(count);
write_bytes_to_xdr_buf(xdr->buf, pathlen_offset, &pathlen, 4);
return 0;
}