On Wed, 2020-10-07 at 17:25 -0400, J. Bruce Fields wrote:
> On Wed, Oct 07, 2020 at 05:07:20PM -0400, trondmy@xxxxxxxxxx wrote:
> > From: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx>
> >
> > If a container sets a net namespace specific uniquifier,
>
> What you're actually checking is if nn->nfs_client != NULL, and
> that's
> pretty much always true. (The only time it's NULL is an allocation
> failure in some initialization code, I think.)
>
> > + struct nfs_net *nn = net_generic(clp->cl_net, nfs_net_id);
> > + struct nfs_netns_client *nn_clp = nn->nfs_client;
> > + const char *id;
> > + size_t len;
> > +
> > buf[0] = '\0';
> > +
> > + if (nn_clp) {
>
> Are you sure you don't mean
>
> if (nn_clp->identifier)
>
> ?
>
> I think that's how you tell if someone's set it.
We're not holding the rcu_read_lock() here, so there is no point in
dereferencing the identifier pointer. That would cause a toctou race...
>
> --b.
>
> > + rcu_read_lock();
> > + id = rcu_dereference(nn_clp->identifier);
> > + if (id && *id != '\0')
> > + len = strlcpy(buf, id, buflen);
> > + rcu_read_unlock();
> > + if (len)
> > + return len;
Oops. However I do need to ensure that 'len' is always initialised
here.
> > + }
> > +
> > if (nfs4_client_id_uniquifier[0] != '\0')
> > return strlcpy(buf, nfs4_client_id_uniquifier, buflen);
> > return 0;
> > @@ -6034,7 +6051,7 @@ nfs4_init_nonuniform_client_string(struct
> > nfs_client *clp)
> > 1;
> > rcu_read_unlock();
> >
> > - buflen = nfs4_get_uniquifier(buf, sizeof(buf));
> > + buflen = nfs4_get_uniquifier(clp, buf, sizeof(buf));
> > if (buflen)
> > len += buflen + 1;
> >
> > @@ -6081,7 +6098,7 @@ nfs4_init_uniform_client_string(struct
> > nfs_client *clp)
> > len = 10 + 10 + 1 + 10 + 1 +
> > strlen(clp->cl_rpcclient->cl_nodename) + 1;
> >
> > - buflen = nfs4_get_uniquifier(buf, sizeof(buf));
> > + buflen = nfs4_get_uniquifier(clp, buf, sizeof(buf));
> > if (buflen)
> > len += buflen + 1;
> >
> > --
> > 2.26.2
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@xxxxxxxxxxxxxxx
