On Tue, Nov 13, 2012 at 05:40:05PM -0500, J. Bruce Fields wrote:
> On Mon, Nov 12, 2012 at 10:17:17AM +0100, Sven Geggus wrote:
> > J. Bruce Fields schrieb am Samstag, den 10. November um 00:24 Uhr:
> >
> > OK, back at work and here is what I get:
> >
> > > Restart the server, start strace, then try the mount, let it hang a few
> > > seconds just to make sure you got anything interesting, then kill strace
> > > and send the output.
> >
> > OK, back at work and here is what I get...
> >
> > read(3, "nfsd 10.1.7.30\n", 2048) = 15
> > close(15) = 0
> > open("/var/lib/nfs/etab", O_RDONLY) = 15
> > close(15) = 0
> > close(15) = 0
> > write(3, "nfsd 10.1.7.30 1352710828 * \n", 29) = 29
> > read(4, "4294967295\n", 2048) = 11
> > close(16) = 0
> > close(15) = 0
> > read(15,
> > "\2\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\377\377\377\377\377\377\377\377\0\0\0\0\0\0\0\0\0\0\0\0",
> > 36) = 36
> > close(15) = 0
> > write(4, "4294967295 1352710828 0 \n", 25) = -1 EINVAL (Invalid argument)
>
> I suspect that error's coming from
> net/sunrpc/svcauth_unix.c:unix_gid_parse().
>
> > 4294967295 is UINT_MAX and this place is where it behaves differently on a good
> > kernel where the write call will succeed:
> >
> > write(4, "4294967295 1352710828 0 \n", 25) = 25
> >
> > Sven
> >
> > P.S.: Your patched svcauth_gss.c will give me an "access denied by server"
> > while mounting instead of the infinite delay:
> > ~/ # mount -t nfs4 -o sec=krb5 testsrv:/storage /mnt/
> > mount.nfs4: access denied by server while mounting testsrv:/storage
>
> So, looks like the same get_int problem exists in several other places.
> Could you try the following instead of the previous patch? I think I
> got them all this time....
Oh, cripes, but this isn't good enough--svcgssd actually passes down -1
id's. Ugh--I'll take a closer look tomorrow.
--b.
>
> --b.
>
> commit 664f26313a738f539a32c4eadd5351905e301bf2
> Author: J. Bruce Fields <bfields@xxxxxxxxxx>
> Date: Fri Nov 9 15:16:02 2012 -0500
>
> svcrpc: fix parsing of uids and gids in gss contexts
>
> bbf43dc888833ac0539e437dbaeb28bfd4fbab9f "sunrpc/cache.h: replace
> simple_strtoul" introduced new range-checking which could cause get_int
> to fail if given an unsigned integer too large to represent as an int.
>
> Symptoms were hangs on krb5 mounts after upgrading an NFS server.
>
> Cc: Eldad Zack <eldad@xxxxxxxxxxxxxxx>
> Reported-by: Sven Geggus <lists@xxxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: J. Bruce Fields <bfields@xxxxxxxxxx>
>
> diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
> index a3946cf..8481961 100644
> --- a/fs/nfsd/export.c
> +++ b/fs/nfsd/export.c
> @@ -491,7 +491,7 @@ static int svc_export_parse(struct cache_detail *cd, char *mesg, int mlen)
> int err;
> struct auth_domain *dom = NULL;
> struct svc_export exp = {}, *expp;
> - int an_int;
> + unsigned int an_int;
>
> if (mesg[mlen-1] != '\n')
> return -EINVAL;
> @@ -531,7 +531,7 @@ static int svc_export_parse(struct cache_detail *cd, char *mesg, int mlen)
> goto out3;
>
> /* flags */
> - err = get_int(&mesg, &an_int);
> + err = get_uint(&mesg, &an_int);
> if (err == -ENOENT) {
> err = 0;
> set_bit(CACHE_NEGATIVE, &exp.h.flags);
> @@ -541,19 +541,19 @@ static int svc_export_parse(struct cache_detail *cd, char *mesg, int mlen)
> exp.ex_flags= an_int;
>
> /* anon uid */
> - err = get_int(&mesg, &an_int);
> + err = get_uint(&mesg, &an_int);
> if (err)
> goto out3;
> exp.ex_anon_uid= an_int;
>
> /* anon gid */
> - err = get_int(&mesg, &an_int);
> + err = get_uint(&mesg, &an_int);
> if (err)
> goto out3;
> exp.ex_anon_gid= an_int;
>
> /* fsid */
> - err = get_int(&mesg, &an_int);
> + err = get_uint(&mesg, &an_int);
> if (err)
> goto out3;
> exp.ex_fsid = an_int;
> diff --git a/fs/nfsd/nfs4idmap.c b/fs/nfsd/nfs4idmap.c
> index a1f10c0..e2c9317 100644
> --- a/fs/nfsd/nfs4idmap.c
> +++ b/fs/nfsd/nfs4idmap.c
> @@ -415,7 +415,7 @@ nametoid_parse(struct cache_detail *cd, char *buf, int buflen)
> goto out;
>
> /* ID */
> - error = get_int(&buf, &ent.id);
> + error = get_uint(&buf, &ent.id);
> if (error == -EINVAL)
> goto out;
> if (error == -ENOENT)
> diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
> index 73e9573..243d180 100644
> --- a/net/sunrpc/auth_gss/svcauth_gss.c
> +++ b/net/sunrpc/auth_gss/svcauth_gss.c
> @@ -444,7 +444,7 @@ static int rsc_parse(struct cache_detail *cd,
> goto out;
>
> /* uid, or NEGATIVE */
> - rv = get_int(&mesg, &rsci.cred.cr_uid);
> + rv = get_uint(&mesg, &rsci.cred.cr_uid);
> if (rv == -EINVAL)
> goto out;
> if (rv == -ENOENT)
> @@ -453,7 +453,7 @@ static int rsc_parse(struct cache_detail *cd,
> int N, i;
>
> /* gid */
> - if (get_int(&mesg, &rsci.cred.cr_gid))
> + if (get_uint(&mesg, &rsci.cred.cr_gid))
> goto out;
>
> /* number of additional gid's */
> @@ -469,7 +469,7 @@ static int rsc_parse(struct cache_detail *cd,
> for (i=0; i<N; i++) {
> gid_t gid;
> kgid_t kgid;
> - if (get_int(&mesg, &gid))
> + if (get_uint(&mesg, &gid))
> goto out;
> kgid = make_kgid(&init_user_ns, gid);
> if (!gid_valid(kgid))
> diff --git a/net/sunrpc/svcauth_unix.c b/net/sunrpc/svcauth_unix.c
> index 4d01292..5d7020a 100644
> --- a/net/sunrpc/svcauth_unix.c
> +++ b/net/sunrpc/svcauth_unix.c
> @@ -493,7 +493,7 @@ static int unix_gid_parse(struct cache_detail *cd,
> return -EINVAL;
> mesg[mlen-1] = 0;
>
> - rv = get_int(&mesg, &uid);
> + rv = get_uint(&mesg, &uid);
> if (rv)
> return -EINVAL;
> ug.uid = uid;
> @@ -513,7 +513,7 @@ static int unix_gid_parse(struct cache_detail *cd,
> for (i = 0 ; i < gids ; i++) {
> int gid;
> kgid_t kgid;
> - rv = get_int(&mesg, &gid);
> + rv = get_uint(&mesg, &gid);
> err = -EINVAL;
> if (rv)
> goto out;
--
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
