On Fri, May 09, 2008 at 05:15:47PM -0700, Benny Halevy wrote:
> On May. 09, 2008, 16:59 -0700, "J. Bruce Fields" <bfields@xxxxxxxxxxxx> wrote:
> > From: J. Bruce Fields <bfields@xxxxxxxxxxxxxx>
> >
> > The code incorrectly assumes here that the server name (or ip address)
> > is null-terminated. This can cause referrals to fail in some cases.
> >
> > Signed-off-by: J. Bruce Fields <bfields@xxxxxxxxxxxxxx>
> > ---
> > fs/nfs/nfs4namespace.c | 12 +++++++++---
> > 1 files changed, 9 insertions(+), 3 deletions(-)
> >
> > I think this was the bug causing the referral failures. There must be a
> > less ugly solution, though.
> >
> > --b.
> >
> > diff --git a/fs/nfs/nfs4namespace.c b/fs/nfs/nfs4namespace.c
> > index 5f9ba41..2684c65 100644
> > --- a/fs/nfs/nfs4namespace.c
> > +++ b/fs/nfs/nfs4namespace.c
> > @@ -96,11 +96,17 @@ static int nfs4_validate_fspath(const struct vfsmount *mnt_parent,
> > /*
> > * Check if the string represents a "valid" IPv4 address
> > */
> > -static inline int valid_ipaddr4(const char *buf)
> > +static inline int valid_ipaddr4(const struct nfs4_string *buf)
> > {
> > int rc, count, in[4];
> >
> > - rc = sscanf(buf, "%d.%d.%d.%d", &in[0], &in[1], &in[2], &in[3]);
> > + /*
> > + * XXX: Depending on the knowledge that the following byte is
> > + * either xdr padding or an xdr length that has already been
> > + * copied:
> > + */
> > + buf->data[buf->len] = '\0';
>
> Hmm, can't that overflow if buf->len is word aligned?
The xdr here is:
struct fs_location4 {
utf8str_cis server<>;
pathname4 rootpath;
};
and we're adding the padding to the end of one of the elements of the
server<> array. So it's guaranteed to be followed by a 4-byte length.
Oh, and we're also keeping the result in a single page, so there's no
crossing of a page boundary.
So it's probably correct. It's too ugly to actually apply, though.
--b.
>
> Benny
>
> > + rc = sscanf(buf->data, "%d.%d.%d.%d", &in[0], &in[1], &in[2], &in[3]);
> > if (rc != 4)
> > return -EINVAL;
> > for (count = 0; count < 4; count++) {
> > @@ -178,7 +184,7 @@ static struct vfsmount *nfs_follow_referral(const struct vfsmount *mnt_parent,
> > };
> >
> > if (location->servers[s].len <= 0 ||
> > - valid_ipaddr4(location->servers[s].data) < 0) {
> > + valid_ipaddr4(&location->servers[s]) < 0) {
> > s++;
> > continue;
> > }
>
>
> --
> Benny Halevy
> Software Architect
> Tel/Fax: +972-3-647-8340
> Mobile: +972-54-802-8340
> US: +1-412-203-3187
> bhalevy@xxxxxxxxxxx
>
> Panasas, Inc.
> Leader in Parallel Storage
> www.panasas.com
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
