On Mon, Aug 26, 2024 at 10:53:25AM +1000, NeilBrown wrote:
> On Sat, 24 Aug 2024, Mike Snitzer wrote:
> > +
> > + /* Save client creds before calling nfsd_file_acquire_local which calls nfsd_setuser */
> > + save_cred = get_current_cred();
>
> I don't think this belongs here. I would rather than
> nfsd_file_acquire_local() saved and restored the cred so it could be
> called without concern for internal implementation details.
OK, will fix.
> > +
> > + /* nfs_fh -> svc_fh */
> > + fh_init(&fh, NFS4_FHSIZE);
> > + fh.fh_handle.fh_size = nfs_fh->size;
> > + memcpy(fh.fh_handle.fh_raw, nfs_fh->data, nfs_fh->size);
> > +
> > + if (fmode & FMODE_READ)
> > + mayflags |= NFSD_MAY_READ;
> > + if (fmode & FMODE_WRITE)
> > + mayflags |= NFSD_MAY_WRITE;
> > +
> > + rpcauth_map_clnt_to_svc_cred_local(rpc_clnt, cred, &rq_cred);
> > +
> > + beres = nfsd_file_acquire_local(cl_nfssvc_net, &rq_cred, rpc_clnt->cl_vers,
> > + cl_nfssvc_dom, &fh, mayflags, pnf);
> > + if (beres) {
> > + status = nfs_stat_to_errno(be32_to_cpu(beres));
> > + goto out_fh_put;
> > + }
> > +out_fh_put:
> > + fh_put(&fh);
> > + if (rq_cred.cr_group_info)
> > + put_group_info(rq_cred.cr_group_info);
> > + revert_creds(save_cred);
> > + nfsd_serv_put(nn);
>
> I think this is too early to be calling nfsd_serv_put().
> I think it should be called when the IO completes - when
> nfs_to.nfsd_file_put() is called.
>
> nfs_to.nfsd_open_local_fh() and nfs_to.nfsd_file_get() should each get a ref to the
> server.
> nfsd_to.nfsd_file_put() should drop the ref.
>
> Note that nfs_do.nfsd_file_get() would not exactly be nfsd_file_get. So
> maybe a different name would suit.
Yes, you're right. I'll get it fixed.
Thanks!
