[PATCH v1] NFS: use TCP for MOUNT when xprtsec= selects TLS

[PATCH v1] NFS: use TCP for MOUNT when xprtsec= selects TLS

[Chuck Lever]

An NFSv2 or NFSv3 text mount that specifies xprtsec= but no
mountproto= crashes in nfs_init_timeout_values():

  RIP: 0010:nfs_init_timeout_values+0x10a/0x110 [nfs]
  Call Trace:
   nfs_mount+0x1c3/0x330 [nfs]
   nfs_try_get_tree+0x1e5/0x460 [nfs]
   nfs_get_tree+0x64d/0x6d0 [nfs]
   vfs_get_tree+0x2b/0xe0
   vfs_cmd_create+0x5f/0xd0
   vfs_fsconfig_locked+0x50/0x130
   __se_sys_fsconfig+0x2f9/0x3a0

nfs_validate_transport_protocol() sets nfs_server.protocol to
XPRT_TRANSPORT_TCP_TLS when xprtsec= is present, but the switch in
nfs_set_mount_transport_protocol() has no case for that value.
mount_server.protocol stays zero, and nfs_init_timeout_values()
calls BUG() on a transport it does not recognize.

mount.nfs always appends mountproto= after probing the server's
rpcbind, so only a caller that bypasses it, such as "mount -i" or a
program that drives fsconfig() directly, reaches the crash.

Select TCP for MOUNT when the NFS transport is XPRT_TRANSPORT_TCP_TLS.
nfs_mount() requests no transport security, so MOUNT already runs
over plain TCP whenever mount.nfs supplies mountproto=tcp.

Fixes: c8407f2e560c ("NFS: Add an "xprtsec=" NFS mount option")
Signed-off-by: Chuck Lever <cel@kernel.org>
---
 fs/nfs/fs_context.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/nfs/fs_context.c b/fs/nfs/fs_context.c
index 1967de7d1dff..a7393ff05494 100644
--- a/fs/nfs/fs_context.c
+++ b/fs/nfs/fs_context.c
@@ -402,6 +402,7 @@ static void nfs_set_mount_transport_protocol(struct nfs_fs_context *ctx)
 		ctx->mount_server.protocol = XPRT_TRANSPORT_UDP;
 		break;
 	case XPRT_TRANSPORT_TCP:
+	case XPRT_TRANSPORT_TCP_TLS:
 	case XPRT_TRANSPORT_RDMA:
 		ctx->mount_server.protocol = XPRT_TRANSPORT_TCP;
 	}
-- 
2.55.0

Re: [v1] NFS: use TCP for MOUNT when xprtsec= selects TLS

[Prabhakar Pujeri]

Hi Chuck,

Reproduced the reasoning against v7.3-rc3. nfs_server.protocol gets set
to XPRT_TRANSPORT_TCP_TLS by xprtsec=, the switch in
nfs_set_mount_transport_protocol() has no case for it, so
mount_server.protocol stays zero and nfs_init_timeout_values() lands in
its default BUG() while setting up the MOUNT client. The one added case
is the smallest correct fix:

- There are no TLS transport variants for RDMA or backchannel, so this
  case covers every selectable TLS value today.
- MOUNT running over plain TCP matches what mount.nfs already arranges
  (it probed rpcbind and appends mountproto=tcp), so no security
  downgrade: MOUNT never runs over TLS either way on NFSv2/v3 text
  mounts.

> 	case XPRT_TRANSPORT_TCP:
> +	case XPRT_TRANSPORT_TCP_TLS:
>  	case XPRT_TRANSPORT_RDMA:

looks right.

Two notes, neither blocking:

- This is a mount-time BUG() reachable via a bare fsconfig() caller
  (mount -i). With Fixes: c8407f2e560c dating to mid-2023, it feels
  worth a Cc: stable@vger.kernel.org.
- As a follow-up, not for this patch: BUG() on an unrecognized
  transport is a harsh way to fail parse-time input. A WARN + error
  would make the next unknown transport friendlier to debug, and keeps
  with the current no-new-BUG() house rule.

Reviewed-by: Prabhakar Pujeri <prabhakar.pujeri@dell.com>