Cmt: [SRU][n:linux-azure][PATCH 1/1] CIFS: New mount option for cifs.upcall namespace resolution
Magali Lemes
magali.lemes at canonical.com
Mon Feb 10 22:39:15 UTC 2025
Questions inline.
On 06/02/2025 16:23, Vinicius Peixoto wrote:
> From: Ritvik Budhiraja <rbudhiraja at microsoft.com>
>
> BugLink: https://bugs.launchpad.net/bugs/2097564
>
> In the current implementation, the SMB filesystem on a mount point can
> trigger upcalls from the kernel to the userspace to enable certain
> functionalities like spnego, dns_resolution, amongst others. These upcalls
> usually either happen in the context of the mount or in the context of an
> application/user. The upcall handler for cifs, cifs.upcall already has
> existing code which switches the namespaces to the caller's namespace
> before handling the upcall. This behaviour is expected for scenarios like
> multiuser mounts, but might not cover all single user scenario with
> services such as Kubernetes, where the mount can happen from different
> locations such as on the host, from an app container, or a driver pod
> which does the mount on behalf of a different pod.
>
> This patch introduces a new mount option called upcall_target, to
> customise the upcall behaviour. upcall_target can take 'mount' and 'app'
> as possible values. This aids use cases like Kubernetes where the mount
> happens on behalf of the application in another container altogether.
> Having this new mount option allows the mount command to specify where the
> upcall should happen: 'mount' for resolving the upcall to the host
> namespace, and 'app' for resolving the upcall to the ns of the calling
> thread. This will enable both the scenarios where the Kerberos credentials
> can be found on the application namespace or the host namespace to which
> just the mount operation is "delegated".
>
> Reviewed-by: Shyam Prasad <shyam.prasad at microsoft.com>
> Reviewed-by: Bharath S M <bharathsm at microsoft.com>
> Reviewed-by: Ronnie Sahlberg <ronniesahlberg at gmail.com>
> Signed-off-by: Ritvik Budhiraja <rbudhiraja at microsoft.com>
> Signed-off-by: Steve French <stfrench at microsoft.com>
> (backported from db363b0a1d9e6b9dc556296f1b1007aeb496a8cf)
> [vpeixoto: fix context conflicts in cifsglob.h, fs_context.h and
> fs_context.c]
> Signed-off-by: Vinicius Peixoto <vinicius.peixoto at canonical.com>
> ---
> fs/smb/client/cifs_spnego.c | 16 +++++++++++++++
> fs/smb/client/cifsfs.c | 25 +++++++++++++++++++++++
> fs/smb/client/cifsglob.h | 25 +++++++++++++++++++++++
> fs/smb/client/connect.c | 20 +++++++++++++++++++
> fs/smb/client/fs_context.c | 40 +++++++++++++++++++++++++++++++++++++
> fs/smb/client/fs_context.h | 11 ++++++++++
> 6 files changed, 137 insertions(+)
>
> diff --git a/fs/smb/client/cifs_spnego.c b/fs/smb/client/cifs_spnego.c
> index af7849e5974f..28f568b5fc27 100644
> --- a/fs/smb/client/cifs_spnego.c
> +++ b/fs/smb/client/cifs_spnego.c
> @@ -82,6 +82,9 @@ struct key_type cifs_spnego_key_type = {
> /* strlen of ";pid=0x" */
> #define PID_KEY_LEN 7
>
> +/* strlen of ";upcall_target=" */
> +#define UPCALL_TARGET_KEY_LEN 15
> +
> /* get a key struct with a SPNEGO security blob, suitable for session setup */
> struct key *
> cifs_get_spnego_key(struct cifs_ses *sesInfo,
> @@ -108,6 +111,11 @@ cifs_get_spnego_key(struct cifs_ses *sesInfo,
> if (sesInfo->user_name)
> desc_len += USER_KEY_LEN + strlen(sesInfo->user_name);
>
> + if (sesInfo->upcall_target == UPTARGET_MOUNT)
> + desc_len += UPCALL_TARGET_KEY_LEN + 5; // strlen("mount")
> + else
> + desc_len += UPCALL_TARGET_KEY_LEN + 3; // strlen("app")
> +
> spnego_key = ERR_PTR(-ENOMEM);
> description = kzalloc(desc_len, GFP_KERNEL);
> if (description == NULL)
> @@ -156,6 +164,14 @@ cifs_get_spnego_key(struct cifs_ses *sesInfo,
> dp = description + strlen(description);
> sprintf(dp, ";pid=0x%x", current->pid);
>
> + if (sesInfo->upcall_target == UPTARGET_MOUNT) {
> + dp = description + strlen(description);
> + sprintf(dp, ";upcall_target=mount");
> + } else {
> + dp = description + strlen(description);
> + sprintf(dp, ";upcall_target=app");
> + }
> +
> cifs_dbg(FYI, "key description = %s\n", description);
> saved_cred = override_creds(spnego_cred);
> spnego_key = request_key(&cifs_spnego_key_type, description, "");
> diff --git a/fs/smb/client/cifsfs.c b/fs/smb/client/cifsfs.c
> index 5e69034076e5..ee29ed5e75fc 100644
> --- a/fs/smb/client/cifsfs.c
> +++ b/fs/smb/client/cifsfs.c
> @@ -534,6 +534,30 @@ static int cifs_show_devname(struct seq_file *m, struct dentry *root)
> return 0;
> }
>
> +static void
> +cifs_show_upcall_target(struct seq_file *s, struct cifs_sb_info *cifs_sb)
> +{
> + if (cifs_sb->ctx->upcall_target == UPTARGET_UNSPECIFIED) {
> + seq_puts(s, ",upcall_target=app");
> + return;
> + }
> +
> + seq_puts(s, ",upcall_target=");
> +
> + switch (cifs_sb->ctx->upcall_target) {
> + case UPTARGET_APP:
> + seq_puts(s, "app");
> + break;
> + case UPTARGET_MOUNT:
> + seq_puts(s, "mount");
> + break;
> + default:
> + /* shouldn't ever happen */
> + seq_puts(s, "unknown");
> + break;
> + }
> +}
> +
> /*
> * cifs_show_options() is for displaying mount options in /proc/mounts.
> * Not all settable options are displayed but most of the important
> @@ -550,6 +574,7 @@ cifs_show_options(struct seq_file *s, struct dentry *root)
> seq_show_option(s, "vers", tcon->ses->server->vals->version_string);
> cifs_show_security(s, tcon->ses);
> cifs_show_cache_flavor(s, cifs_sb);
> + cifs_show_upcall_target(s, cifs_sb);
>
> if (tcon->no_lease)
> seq_puts(s, ",nolease");
> diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
> index d32597a4d4f3..e035f1ac5d3b 100644
> --- a/fs/smb/client/cifsglob.h
> +++ b/fs/smb/client/cifsglob.h
> @@ -153,6 +153,30 @@ enum securityEnum {
> Kerberos, /* Kerberos via SPNEGO */
> };
>
> +enum upcall_target_enum {
> + UPTARGET_UNSPECIFIED, /* not specified, defaults to app */
> + UPTARGET_MOUNT, /* upcall to the mount namespace */
> + UPTARGET_APP, /* upcall to the application namespace which did the mount */
> +};
> +
> +enum cifs_reparse_type {
> + CIFS_REPARSE_TYPE_NFS,
> + CIFS_REPARSE_TYPE_WSL,
> + CIFS_REPARSE_TYPE_DEFAULT = CIFS_REPARSE_TYPE_NFS,
> +};
> +
> +static inline const char *cifs_reparse_type_str(enum cifs_reparse_type type)
> +{
> + switch (type) {
> + case CIFS_REPARSE_TYPE_NFS:
> + return "nfs";
> + case CIFS_REPARSE_TYPE_WSL:
> + return "wsl";
> + default:
> + return "unknown";
> + }
> +}
> +
Why is cifs_reparse_type_str() being backported (as well as
cifs_reparse_type, only used by cifs_reparse_type_str())? It doesn't
seem to be used anywhere.
> struct session_key {
> unsigned int len;
> char *response;
> @@ -1055,6 +1079,7 @@ struct cifs_ses {
> struct session_key auth_key;
> struct ntlmssp_auth *ntlmssp; /* ciphertext, flags, server challenge */
> enum securityEnum sectype; /* what security flavor was specified? */
> + enum upcall_target_enum upcall_target; /* what upcall target was specified? */
> bool sign; /* is signing required? */
> bool domainAuto:1;
> bool expired_pwd; /* track if access denied or expired pwd so can know if need to update */
> diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c
> index 07363268d480..c4dd57d20416 100644
> --- a/fs/smb/client/connect.c
> +++ b/fs/smb/client/connect.c
> @@ -2343,6 +2343,26 @@ cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb3_fs_context *ctx)
>
> ses->sectype = ctx->sectype;
> ses->sign = ctx->sign;
> +
> + /*
> + *Explicitly marking upcall_target mount option for easier handling
> + * by cifs_spnego.c and eventually cifs.upcall.c
> + */
> +
> + switch (ctx->upcall_target) {
> + case UPTARGET_UNSPECIFIED: /* default to app */
> + case UPTARGET_APP:
> + ses->upcall_target = UPTARGET_APP;
> + break;
> + case UPTARGET_MOUNT:
> + ses->upcall_target = UPTARGET_MOUNT;
> + break;
> + default:
> + // should never happen
> + ses->upcall_target = UPTARGET_APP;
> + break;
> + }
> +
> ses->local_nls = load_nls(ctx->local_nls->charset);
>
> /* add server as first channel */
> diff --git a/fs/smb/client/fs_context.c b/fs/smb/client/fs_context.c
> index f119035a8272..def31c3ef4e4 100644
> --- a/fs/smb/client/fs_context.c
> +++ b/fs/smb/client/fs_context.c
> @@ -67,6 +67,12 @@ static const match_table_t cifs_secflavor_tokens = {
> { Opt_sec_err, NULL }
> };
>
> +static const match_table_t cifs_upcall_target = {
> + { Opt_upcall_target_mount, "mount" },
> + { Opt_upcall_target_application, "app" },
> + { Opt_upcall_target_err, NULL }
> +};
> +
> const struct fs_parameter_spec smb3_fs_parameters[] = {
> /* Mount options that take no arguments */
> fsparam_flag_no("user_xattr", Opt_user_xattr),
> @@ -175,6 +181,8 @@ const struct fs_parameter_spec smb3_fs_parameters[] = {
> fsparam_string("vers", Opt_vers),
> fsparam_string("sec", Opt_sec),
> fsparam_string("cache", Opt_cache),
> + fsparam_string("reparse", Opt_reparse),
Is this right? I think this may partially introduce a mount option which
doesn't have full support as the other pieces haven't been fully backported.
> + fsparam_string("upcall_target", Opt_upcalltarget),
>
> /* Arguments that should be ignored */
> fsparam_flag("guest", Opt_ignore),
> @@ -245,6 +253,29 @@ cifs_parse_security_flavors(struct fs_context *fc, char *value, struct smb3_fs_c
> return 0;
> }
>
> +static int
> +cifs_parse_upcall_target(struct fs_context *fc, char *value, struct smb3_fs_context *ctx)
> +{
> + substring_t args[MAX_OPT_ARGS];
> +
> + ctx->upcall_target = UPTARGET_UNSPECIFIED;
> +
> + switch (match_token(value, cifs_upcall_target, args)) {
> + case Opt_upcall_target_mount:
> + ctx->upcall_target = UPTARGET_MOUNT;
> + break;
> + case Opt_upcall_target_application:
> + ctx->upcall_target = UPTARGET_APP;
> + break;
> +
> + default:
> + cifs_errorf(fc, "bad upcall target: %s\n", value);
> + return 1;
> + }
> +
> + return 0;
> +}
> +
> static const match_table_t cifs_cacheflavor_tokens = {
> { Opt_cache_loose, "loose" },
> { Opt_cache_strict, "strict" },
> @@ -1425,6 +1456,10 @@ static int smb3_fs_context_parse_param(struct fs_context *fc,
> if (cifs_parse_security_flavors(fc, param->string, ctx) != 0)
> goto cifs_parse_mount_err;
> break;
> + case Opt_upcalltarget:
> + if (cifs_parse_upcall_target(fc, param->string, ctx) != 0)
> + goto cifs_parse_mount_err;
> + break;
> case Opt_cache:
> if (cifs_parse_cache_flavor(fc, param->string, ctx) != 0)
> goto cifs_parse_mount_err;
> @@ -1598,6 +1633,11 @@ static int smb3_fs_context_parse_param(struct fs_context *fc,
> }
> /* case Opt_ignore: - is ignored as expected ... */
>
> + if (ctx->multiuser && ctx->upcall_target == UPTARGET_MOUNT) {
> + cifs_errorf(fc, "multiuser mount option not supported with upcalltarget set as 'mount'\n");
> + goto cifs_parse_mount_err;
> + }
> +
> return 0;
>
> cifs_parse_mount_err:
> diff --git a/fs/smb/client/fs_context.h b/fs/smb/client/fs_context.h
> index 369a3fea1dfe..ca48a62767ae 100644
> --- a/fs/smb/client/fs_context.h
> +++ b/fs/smb/client/fs_context.h
> @@ -54,6 +54,12 @@ enum cifs_sec_param {
> Opt_sec_err
> };
>
> +enum cifs_upcall_target_param {
> + Opt_upcall_target_mount,
> + Opt_upcall_target_application,
> + Opt_upcall_target_err
> +};
> +
> enum cifs_param {
> /* Mount options that take no arguments */
> Opt_user_xattr,
> @@ -107,6 +113,8 @@ enum cifs_param {
> Opt_multichannel,
> Opt_compress,
> Opt_witness,
> + Opt_is_upcall_target_mount,
> + Opt_is_upcall_target_application,
>
> /* Mount options which take numeric value */
> Opt_backupuid,
> @@ -149,6 +157,8 @@ enum cifs_param {
> Opt_vers,
> Opt_sec,
> Opt_cache,
> + Opt_reparse,
> + Opt_upcalltarget,
>
> /* Mount options to be ignored */
> Opt_ignore,
> @@ -190,6 +200,7 @@ struct smb3_fs_context {
> umode_t file_mode;
> umode_t dir_mode;
> enum securityEnum sectype; /* sectype requested via mnt opts */
> + enum upcall_target_enum upcall_target; /* where to upcall for mount */
> bool sign; /* was signing requested via mnt opts? */
> bool ignore_signature:1;
> bool retry:1;
More information about the kernel-team
mailing list