Hi Linus,
The following changes since commit abc13275771fac77e2d7b129c289522dacb644b6:
SUNRPC: Remove xprt_connect_status() (2018-12-18 11:04:10 -0500)
are available in the Git repository at:
git://git.linux-nfs.org/projects/anna/linux-nfs.git tags/nfs-for-4.21-1
for you to fetch changes up to 260f71eff493a844531629854c0935fa8de4fa2c:
sunrpc: convert to DEFINE_SHOW_ATTRIBUTE (2019-01-02 12:05:49 -0500)
----------------------------------------------------------------
Note that there is a conflict with the rdma tree in this pull request, since
we delete a file that has been changed in the rdma tree. Hopefully that's
easy enough to resolve!
We also were unable to track down a maintainer for Neil Brown's changes to
the generic cred code that are prerequisites to his RPC cred cleanup patches.
We've been asking around for several months without any response, so
hopefully it's okay to include those patches in this pull request.
Stable bugfixes:
- xprtrdma: Yet another double DMA-unmap # v4.20
Features:
- Allow some /proc/sys/sunrpc entries without CONFIG_SUNRPC_DEBUG
- Per-xprt rdma receive workqueues
- Drop support for FMR memory registration
- Make port= mount option optional for RDMA mounts
Other bugfixes and cleanups:
- Remove unused nfs4_xdev_fs_type declaration
- Fix comments for behavior that has changed
- Remove generic RPC credentials by switching to 'struct cred'
- Fix crossing mountpoints with different auth flavors
- Various xprtrdma fixes from testing and auditing the close code
- Fixes for disconnect issues when using xprtrdma with krb5
- Clean up and improve xprtrdma trace points
- Fix NFS v4.2 async copy reboot recovery
Thanks,
Anna
----------------------------------------------------------------
Ben Dooks (1):
SUNRPC: allow /proc entries without CONFIG_SUNRPC_DEBUG
Chris Perl (1):
NFS: nfs_compare_mount_options always compare auth flavors.
Chuck Lever (30):
xprtrdma: Yet another double DMA-unmap
xprtrdma: Ensure MRs are DMA-unmapped when posting LOCAL_INV fails
xprtrdma: Refactor Receive accounting
xprtrdma: Replace rpcrdma_receive_wq with a per-xprt workqueue
xprtrdma: No qp_event disconnect
xprtrdma: Don't wake pending tasks until disconnect is done
xprtrdma: Fix ri_max_segs and the result of ro_maxpages
xprtrdma: Reduce max_frwr_depth
xprtrdma: Remove support for FMR memory registration
xprtrdma: Remove rpcrdma_memreg_ops
xprtrdma: Plant XID in on-the-wire RDMA offset (FRWR)
NFS: Make "port=" mount option optional for RDMA mounts
xprtrdma: Recognize XDRBUF_SPARSE_PAGES
xprtrdma: Remove request_module from backchannel
xprtrdma: Expose transport header errors
xprtrdma: Simplify locking that protects the rl_allreqs list
xprtrdma: Cull dprintk() call sites
xprtrdma: Remove unused fields from rpcrdma_ia
xprtrdma: Clean up of xprtrdma chunk trace points
xprtrdma: Relocate the xprtrdma_mr_map trace points
xprtrdma: Add trace points for calls to transport switch methods
xprtrdma: Trace mapping, alloc, and dereg failures
NFS: Fix NFSv4 symbolic trace point output
SUNRPC: Simplify defining common RPC trace events
SUNRPC: Fix some kernel doc complaints
xprtrdma: Update comments in frwr_op_send
xprtrdma: Replace outdated comment for rpcrdma_ep_post
xprtrdma: Add documenting comment for rpcrdma_buffer_destroy
xprtrdma: Don't leak freed MRs
xprtrdma: Prevent leak of rpcrdma_rep objects
J. Bruce Fields (2):
sunrpc: handle ENOMEM in rpcb_getport_async
sunrpc: convert unnecessary GFP_ATOMIC to GFP_NOFS
NeilBrown (24):
cred: add cred_fscmp() for comparing creds.
cred: add get_cred_rcu()
cred: export get_task_cred().
cred: allow get_cred() and put_cred() to be given NULL.
SUNRPC: add 'struct cred *' to auth_cred and rpc_cred
SUNRPC: remove groupinfo from struct auth_cred.
SUNRPC: remove uid and gid from struct auth_cred
SUNRPC: remove machine_cred field from struct auth_cred
NFSv4: add cl_root_cred for use when machine cred is not available.
NFSv4: don't require lock for get_renew_cred or get_machine_cred
SUNRPC: discard RPC_DO_ROOTOVERRIDE()
NFS/SUNRPC: don't lookup machine credential until rpcauth_bindcred().
SUNRPC: introduce RPC_TASK_NULLCREDS to request auth_none
SUNRPC: add side channel to use non-generic cred for rpc call.
NFS: move credential expiry tracking out of SUNRPC into NFS.
SUNRPC: remove RPCAUTH_AUTH_NO_CRKEY_TIMEOUT
NFS: change access cache to use 'struct cred'.
NFS: struct nfs_open_dir_context: convert rpc_cred pointer to cred.
NFS/NFSD/SUNRPC: replace generic creds with 'struct cred'.
SUNRPC: remove generic cred code.
SUNRPC: remove crbind rpc_cred operation
SUNRPC: simplify auth_unix.
SUNRPC discard cr_uid from struct rpc_cred.
NFS: remove unnecessary test for IS_ERR(cred)
Olga Kornievskaia (2):
NFSv4: cleanup remove unused nfs4_xdev_fs_type
NFSv4.2 fix async copy reboot recovery
Pavel Tikhomirov (1):
nfs: fix comment to nfs_generic_pg_test which does the opposite
Santosh kumar pradhan (1):
sunrpc: Add xprt after nfs4_test_session_trunk()
Yangtao Li (1):
sunrpc: convert to DEFINE_SHOW_ATTRIBUTE
fs/lockd/clntproc.c | 6 +-
fs/nfs/blocklayout/blocklayout.c | 2 +-
fs/nfs/client.c | 9 +-
fs/nfs/delegation.c | 28 +-
fs/nfs/delegation.h | 10 +-
fs/nfs/dir.c | 59 ++--
fs/nfs/flexfilelayout/flexfilelayout.c | 64 ++--
fs/nfs/flexfilelayout/flexfilelayout.h | 8 +-
fs/nfs/flexfilelayout/flexfilelayoutdev.c | 16 +-
fs/nfs/inode.c | 13 +-
fs/nfs/internal.h | 15 +-
fs/nfs/nfs3proc.c | 4 +-
fs/nfs/nfs4_fs.h | 68 ++---
fs/nfs/nfs4client.c | 4 +-
fs/nfs/nfs4proc.c | 158 +++++-----
fs/nfs/nfs4renewd.c | 9 +-
fs/nfs/nfs4session.c | 5 +-
fs/nfs/nfs4state.c | 131 ++++-----
fs/nfs/nfs4trace.h | 456 ++++++++++++++++++++---------
fs/nfs/pagelist.c | 4 +-
fs/nfs/pnfs.c | 14 +-
fs/nfs/pnfs.h | 10 +-
fs/nfs/pnfs_dev.c | 4 +-
fs/nfs/pnfs_nfs.c | 2 +-
fs/nfs/proc.c | 2 +-
fs/nfs/super.c | 13 +-
fs/nfs/unlink.c | 20 +-
fs/nfs/write.c | 24 +-
fs/nfsd/nfs4callback.c | 31 +-
fs/nfsd/state.h | 2 +-
include/linux/cred.h | 26 +-
include/linux/nfs_fs.h | 13 +-
include/linux/nfs_fs_sb.h | 2 +-
include/linux/nfs_xdr.h | 16 +-
include/linux/sunrpc/auth.h | 51 +---
include/linux/sunrpc/clnt.h | 5 +-
include/linux/sunrpc/sched.h | 6 +-
include/trace/events/rpcrdma.h | 218 ++++++++++++--
include/trace/events/sunrpc.h | 172 +++++------
kernel/cred.c | 58 +++-
net/sunrpc/Makefile | 2 +-
net/sunrpc/auth.c | 116 ++++----
net/sunrpc/auth_generic.c | 293 ------------------
net/sunrpc/auth_gss/auth_gss.c | 47 +--
net/sunrpc/auth_gss/gss_mech_switch.c | 2 +-
net/sunrpc/auth_null.c | 4 -
net/sunrpc/auth_unix.c | 110 +++----
net/sunrpc/backchannel_rqst.c | 2 +-
net/sunrpc/clnt.c | 29 +-
net/sunrpc/rpc_pipe.c | 19 +-
net/sunrpc/rpcb_clnt.c | 12 +-
net/sunrpc/sched.c | 5 +-
net/sunrpc/xprtmultipath.c | 4 +-
net/sunrpc/xprtrdma/Makefile | 3 +-
net/sunrpc/xprtrdma/backchannel.c | 39 +--
net/sunrpc/xprtrdma/fmr_ops.c | 337 ---------------------
net/sunrpc/xprtrdma/frwr_ops.c | 209 ++++++++-----
net/sunrpc/xprtrdma/rpc_rdma.c | 78 +++--
net/sunrpc/xprtrdma/svc_rdma_backchannel.c | 8 +-
net/sunrpc/xprtrdma/transport.c | 91 +++---
net/sunrpc/xprtrdma/verbs.c | 255 +++++++---------
net/sunrpc/xprtrdma/xprt_rdma.h | 80 ++---
net/sunrpc/xprtsock.c | 10 +-
63 files changed, 1518 insertions(+), 1995 deletions(-)
delete mode 100644 net/sunrpc/auth_generic.c
delete mode 100644 net/sunrpc/xprtrdma/fmr_ops.c
On Wed, Jan 2, 2019 at 2:42 PM Schumaker, Anna
<[email protected]> wrote:
>
> We also were unable to track down a maintainer for Neil Brown's changes to
> the generic cred code that are prerequisites to his RPC cred cleanup patches.
> We've been asking around for several months without any response, so
> hopefully it's okay to include those patches in this pull request.
Looks ok to me, although I wonder what the semantics of cred_fscmp()
are across namespaces?
IOW, it seems potentially a bit suspicious to do cred_fscmp() if the
two creds have different namnespaces? Hmm?
Is there some reason that can't happen, or some reason it doesn't matter?
Linus
The pull request you sent on Wed, 2 Jan 2019 22:42:32 +0000:
> git://git.linux-nfs.org/projects/anna/linux-nfs.git tags/nfs-for-4.21-1
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/e6b92572808467f35fd159d47c45b650de29e722
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker
On Wed, Jan 02 2019, Linus Torvalds wrote:
> On Wed, Jan 2, 2019 at 2:42 PM Schumaker, Anna
> <[email protected]> wrote:
>>
>> We also were unable to track down a maintainer for Neil Brown's changes to
>> the generic cred code that are prerequisites to his RPC cred cleanup patches.
>> We've been asking around for several months without any response, so
>> hopefully it's okay to include those patches in this pull request.
>
> Looks ok to me, although I wonder what the semantics of cred_fscmp()
> are across namespaces?
>
> IOW, it seems potentially a bit suspicious to do cred_fscmp() if the
> two creds have different namnespaces? Hmm?
>
> Is there some reason that can't happen, or some reason it doesn't matter?
>
> Linus
Interesting question.
For the current use in NFS, it is consistent with existing practice to
ignore the name space.
NFS file accesses (when using the normal uid-based access checks) always
use the manifest uid of the process - the one returned by getuid() (or
more accurately, getfsuid()).
Maybe this is wrong? Maybe we should always use from_kuid() or whatever
to get the uid/gid to send over the wire?
Anna/Trond: do you have thoughts on this? If a process in a user
namespace accesses a file over NFS, should the UID presented to the
server be the one in that name-space, or the one you get by mapping to
the global name-space?
Or should we map to the namespace that was active when the filesystem
was mounted?
I don't think cred_fscmp() should do any of this mapping, but maybe it
should treat creds from different namespaces as different - as a
precaution.
Thanks,
NeilBrown
On Thu, 2019-01-03 at 15:53 +1100, NeilBrown wrote:
> On Wed, Jan 02 2019, Linus Torvalds wrote:
>
> > On Wed, Jan 2, 2019 at 2:42 PM Schumaker, Anna
> > <[email protected]> wrote:
> > > We also were unable to track down a maintainer for Neil Brown's
> > > changes to
> > > the generic cred code that are prerequisites to his RPC cred
> > > cleanup patches.
> > > We've been asking around for several months without any response,
> > > so
> > > hopefully it's okay to include those patches in this pull
> > > request.
> >
> > Looks ok to me, although I wonder what the semantics of
> > cred_fscmp()
> > are across namespaces?
> >
> > IOW, it seems potentially a bit suspicious to do cred_fscmp() if
> > the
> > two creds have different namnespaces? Hmm?
> >
> > Is there some reason that can't happen, or some reason it doesn't
> > matter?
> >
> > Linus
>
> Interesting question.
> For the current use in NFS, it is consistent with existing practice
> to
> ignore the name space.
> NFS file accesses (when using the normal uid-based access checks)
> always
> use the manifest uid of the process - the one returned by getuid()
> (or
> more accurately, getfsuid()).
> Maybe this is wrong? Maybe we should always use from_kuid() or
> whatever
> to get the uid/gid to send over the wire?
>
> Anna/Trond: do you have thoughts on this? If a process in a user
> namespace accesses a file over NFS, should the UID presented to the
> server be the one in that name-space, or the one you get by mapping
> to
> the global name-space?
> Or should we map to the namespace that was active when the filesystem
> was mounted?
>
> I don't think cred_fscmp() should do any of this mapping, but maybe
> it
> should treat creds from different namespaces as different - as a
> precaution.
>
> Thanks,
> NeilBrown
The values being compared are in cred_fscmp() are all of type kuid_t or
kgid_t so that means they have already been mapped from the user
namespace into the kernel uid/gid space.
When we put those kuid/kgid values onto the wire, we currently always
use the init namespace rather than the user namespace of the mount
process.
When using strong authentication (i.e. krb5) then none of this matters,
since the server performs its own mapping of the presented RPCSEC_GSS
session into a credential. That mapping is independent of the user
namespace on the client, it just depends on which krb5 principal the
process used to identify itself.
The problem case is limited to when we're using the weak AUTH_UNIX
authentication, since the server is then implicitly trusting the client
to protect against identity spoofing. This is particularly true if the
NFS server is being accessed through NAT, in which case it has very
limited possibilities for discriminating between containers on the same
client using the export table because they will all originate from the
same source IP address. I think that for these cases, using the init
namespace is the right thing to do for the same reason we use it with
local filesystems: if we try to use a different namespace then
unprivileged userspace processes might be able to manipulate the
mapping to spoof the identities of privileged users or groups, or
otherwise gain access to files to which they normally should not have
access.
Does that argument make sense?
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
[email protected]
Trond Myklebust <[email protected]> writes:
> On Thu, 2019-01-03 at 15:53 +1100, NeilBrown wrote:
>> On Wed, Jan 02 2019, Linus Torvalds wrote:
>>
>> > On Wed, Jan 2, 2019 at 2:42 PM Schumaker, Anna
>> > <[email protected]> wrote:
>> > > We also were unable to track down a maintainer for Neil Brown's
>> > > changes to
>> > > the generic cred code that are prerequisites to his RPC cred
>> > > cleanup patches.
>> > > We've been asking around for several months without any response,
>> > > so
>> > > hopefully it's okay to include those patches in this pull
>> > > request.
>> >
>> > Looks ok to me, although I wonder what the semantics of
>> > cred_fscmp()
>> > are across namespaces?
>> >
>> > IOW, it seems potentially a bit suspicious to do cred_fscmp() if
>> > the
>> > two creds have different namnespaces? Hmm?
>> >
>> > Is there some reason that can't happen, or some reason it doesn't
>> > matter?
I think the function might be better named cred_uidgid_cmp to make
it clear that the comparison only cares about the uid and gid values
in the cred. As Trond points out all of the values are of kuid_t
and kgid_t so are namespace independent at that point.
>> Interesting question.
>> For the current use in NFS, it is consistent with existing practice
>> to
>> ignore the name space.
>> NFS file accesses (when using the normal uid-based access checks)
>> always
>> use the manifest uid of the process - the one returned by getuid()
>> (or
>> more accurately, getfsuid()).
>> Maybe this is wrong? Maybe we should always use from_kuid() or
>> whatever
>> to get the uid/gid to send over the wire?
>>
>> Anna/Trond: do you have thoughts on this? If a process in a user
>> namespace accesses a file over NFS, should the UID presented to the
>> server be the one in that name-space, or the one you get by mapping
>> to
>> the global name-space?
>> Or should we map to the namespace that was active when the filesystem
>> was mounted?
>>
>> I don't think cred_fscmp() should do any of this mapping, but maybe
>> it
>> should treat creds from different namespaces as different - as a
>> precaution.
>>
>> Thanks,
>> NeilBrown
>
> The values being compared are in cred_fscmp() are all of type kuid_t or
> kgid_t so that means they have already been mapped from the user
> namespace into the kernel uid/gid space.
> When we put those kuid/kgid values onto the wire, we currently always
> use the init namespace rather than the user namespace of the mount
> process.
It happens that mounts of nfs are still limited to mounter in the
initial user namespace. While it seems plausible that we could allow
unprivileged users to mount nfs filesystems the auditing of the code
base has not been done to confirm that is safe. The nfs code base is a
large attack surface so this something that must be done with quite a
bit of caution.
> When using strong authentication (i.e. krb5) then none of this matters,
> since the server performs its own mapping of the presented RPCSEC_GSS
> session into a credential. That mapping is independent of the user
> namespace on the client, it just depends on which krb5 principal the
> process used to identify itself.
>
> The problem case is limited to when we're using the weak AUTH_UNIX
> authentication, since the server is then implicitly trusting the client
> to protect against identity spoofing. This is particularly true if the
> NFS server is being accessed through NAT, in which case it has very
> limited possibilities for discriminating between containers on the same
> client using the export table because they will all originate from the
> same source IP address. I think that for these cases, using the init
> namespace is the right thing to do for the same reason we use it with
> local filesystems: if we try to use a different namespace then
> unprivileged userspace processes might be able to manipulate the
> mapping to spoof the identities of privileged users or groups, or
> otherwise gain access to files to which they normally should not have
> access.
>
> Does that argument make sense?
I think the NAT case is something to be concerned about. However I
believe the same attack is possible with an ordinary UDP or TCP socket
bound for the same server. So I don't know if we can protect the
server.
In general I believe we should put things on the wire that match
sb->s_user_ns. Matching the mounter of the filesystem. Because that
means behavior on the wire is independent of if you are running
in a container or not. That seems to be the least surprising thing to
do, and would match what a userspace nfs client would do. But again
that is not an immediate concern.
Eric
On Wed, Jan 2, 2019 at 10:10 PM Trond Myklebust <[email protected]> wrote:
>
> The values being compared are in cred_fscmp() are all of type kuid_t or
> kgid_t so that means they have already been mapped from the user
> namespace into the kernel uid/gid space.
Ok, "everything happens in init_ns" is I guess a very valid argument.
It might be good to perhaps show that in the name (and maybe even a
comment), but as long as it's only really used for nfs I guess it
isn't a big deal.
Thanks,
Linus