From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from air.basealt.ru (air.basealt.ru [193.43.8.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 07B0FB67A for ; Thu, 3 Jul 2025 13:05:03 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=193.43.8.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1751547907; cv=none; b=bTl85CiN8ZakFbu0G35LM6TS55ogZwlS14cY2kY/VFYbDBOw0Z9bcr4PkNc4toK+NbqkNKzIPvXHeDj2nH3/kIQneh7NAf4ZOVyFINZCtmEehHIy7ewV14WJEQdNvjBE73b+L0B4RMgmKYnjO2bC0P7X1jCpmo7ERfPvzgmWQFE= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1751547907; c=relaxed/simple; bh=2vVYB0sTOj/Biqrp8wdfLAIK8S2bNIxIoTIE2ZBwob0=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=WsXZW9EBbCUKxo4njzSjtrICuSgKDgIVDjVDW/NVi99LSvYIdrvJYeaeYqU4VPHRa/2lxm64IcHwFLNivSz9/d9Ewrvj6YXWnFlniUZb1zK89B94gVVYfyihD7KPzn8Hf6h654csY15DUnDoL6qscL0h34S3Doq4wpUzKfzZkpA= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=altlinux.org; spf=pass smtp.mailfrom=altlinux.org; arc=none smtp.client-ip=193.43.8.18 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=altlinux.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=altlinux.org Received: from [10.64.129.108] (unknown [193.43.9.250]) (Authenticated sender: alekseevamo) by air.basealt.ru (Postfix) with ESMTPSA id 86EA8233B3; Thu, 3 Jul 2025 15:59:32 +0300 (MSK) Message-ID: <3d3160fd-e29d-495d-a02e-e28558cfec1a@altlinux.org> Date: Thu, 3 Jul 2025 16:59:32 +0400 Precedence: bulk X-Mailing-List: linux-cifs@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Reply-To: alxvmr@altlinux.org Subject: Re: [PATCH] fs/smb/client/fs_context: Add hostname option for CIFS module to work with domain-based dfs resources with Kerberos authentication To: Vitaly Chikunov Cc: linux-cifs@vger.kernel.org, samba-technical@lists.samba.org, sfrench@samba.org, pc@manguebit.com, Ivan Volchenko References: <20250516152201.201385-1-alxvmr@altlinux.org> <43os6kphihnry2wggqykiwmusz@pony.office.basealt.ru> Content-Language: en-US, ru From: Maria Alexeeva In-Reply-To: <43os6kphihnry2wggqykiwmusz@pony.office.basealt.ru> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit On 6/14/25 07:42, Vitaly Chikunov wrote: > Maria, > > On Fri, May 16, 2025 at 07:22:01PM +0400, Maria Alexeeva wrote: >> Paths to domain-based dfs resources are defined using the domain name >> of the server in the format: >> \\DOMAIN.NAME>\\ >> >> The CIFS module, when requesting a TGS, uses the server name >> () it obtained from the UNC for the initial connection. >> It then composes an SPN that does not match any entities >> in the domain because it is the domain name itself. > For a casual reader like me it's hard to understand (this abbreviation > filled message) what it's all about. And why we can't just change system > hostname for example. This option is needed to transfer the real name of the server to which the connection is taking place, when using the UNC path in the form of domain-based DFS. The system hostname has nothing to do with it. > Also, the summary (subject) message is 180 character which is way above > 75 characters suggested in submitting-patches.rst. > >> To eliminate this behavior, a hostname option is added, which is >> the name of the server to connect to and is used in composing the SPN. >> In the future this option will be used in the cifs-utils development. >> >> Suggested-by: Ivan Volchenko >> Signed-off-by: Maria Alexeeva >> --- >> fs/smb/client/fs_context.c | 35 +++++++++++++++++++++++++++++------ >> fs/smb/client/fs_context.h | 3 +++ >> 2 files changed, 32 insertions(+), 6 deletions(-) >> >> diff --git a/fs/smb/client/fs_context.c b/fs/smb/client/fs_context.c >> index a634a34d4086..74de0a9de664 100644 >> --- a/fs/smb/client/fs_context.c >> +++ b/fs/smb/client/fs_context.c >> @@ -177,6 +177,7 @@ const struct fs_parameter_spec smb3_fs_parameters[] = { >> fsparam_string("password2", Opt_pass2), >> fsparam_string("ip", Opt_ip), >> fsparam_string("addr", Opt_ip), >> + fsparam_string("hostname", Opt_hostname), >> fsparam_string("domain", Opt_domain), >> fsparam_string("dom", Opt_domain), >> fsparam_string("srcaddr", Opt_srcaddr), >> @@ -825,16 +826,23 @@ static int smb3_fs_context_validate(struct fs_context *fc) >> return -ENOENT; >> } >> >> + if (ctx->got_opt_hostname) { >> + kfree(ctx->server_hostname); >> + ctx->server_hostname = ctx->opt_hostname; > I am not familiar with the smb codebase but are you sure this will not > cause a race? The race condition will not occur. ctx->server_hostname is also used in smb3_parse_devname inside smb3_fs_context_parse_param. smb3_fs_context_parse_param is called earlier than the updated smb3_fs_context_validate, which is called inside smb3_get_tree: static const struct fs_context_operations smb3_fs_context_ops = {  .free   = smb3_fs_context_free,  .parse_param  = smb3_fs_context_parse_param,  .parse_monolithic = smb3_fs_context_parse_monolithic,  .get_tree  = smb3_get_tree,  .reconfigure  = smb3_reconfigure, }; >> + pr_notice("changing server hostname to name provided in hostname= option\n"); >> + } >> + >> if (!ctx->got_ip) { >> int len; >> - const char *slash; >> >> - /* No ip= option specified? Try to get it from UNC */ >> - /* Use the address part of the UNC. */ >> - slash = strchr(&ctx->UNC[2], '\\'); >> - len = slash - &ctx->UNC[2]; >> + /* >> + * No ip= option specified? Try to get it from server_hostname >> + * Use the address part of the UNC parsed into server_hostname >> + * or hostname= option if specified. >> + */ >> + len = strlen(ctx->server_hostname); >> if (!cifs_convert_address((struct sockaddr *)&ctx->dstaddr, >> - &ctx->UNC[2], len)) { >> + ctx->server_hostname, len)) { >> pr_err("Unable to determine destination address\n"); >> return -EHOSTUNREACH; >> } >> @@ -1518,6 +1526,21 @@ static int smb3_fs_context_parse_param(struct fs_context *fc, >> } >> ctx->got_ip = true; >> break; >> + case Opt_hostname: >> + if (strnlen(param->string, CIFS_NI_MAXHOST) == CIFS_NI_MAXHOST) { >> + pr_warn("host name too long\n"); >> + goto cifs_parse_mount_err; >> + } >> + >> + kfree(ctx->opt_hostname); >> + ctx->opt_hostname = kstrdup(param->string, GFP_KERNEL); >> + if (ctx->opt_hostname == NULL) { >> + cifs_errorf(fc, "OOM when copying hostname string\n"); >> + goto cifs_parse_mount_err; >> + } >> + cifs_dbg(FYI, "Host name set\n"); >> + ctx->got_opt_hostname = true; >> + break; >> case Opt_domain: >> if (strnlen(param->string, CIFS_MAX_DOMAINNAME_LEN) >> == CIFS_MAX_DOMAINNAME_LEN) { >> diff --git a/fs/smb/client/fs_context.h b/fs/smb/client/fs_context.h >> index 9e83302ce4b8..cf0478b1eff9 100644 >> --- a/fs/smb/client/fs_context.h >> +++ b/fs/smb/client/fs_context.h >> @@ -184,6 +184,7 @@ enum cifs_param { >> Opt_pass, >> Opt_pass2, >> Opt_ip, >> + Opt_hostname, >> Opt_domain, >> Opt_srcaddr, >> Opt_iocharset, >> @@ -214,6 +215,7 @@ struct smb3_fs_context { >> bool gid_specified; >> bool sloppy; >> bool got_ip; >> + bool got_opt_hostname; >> bool got_version; >> bool got_rsize; >> bool got_wsize; >> @@ -226,6 +228,7 @@ struct smb3_fs_context { >> char *domainname; >> char *source; >> char *server_hostname; >> + char *opt_hostname; > Perhaps, smb3_fs_context_dup and smb3_cleanup_fs_context_contents should > be aware of these new fields too. smb3_cleanup_fs_context_contents should be aware of these new fields too. Clearing in smb3_cleanup_fs_context_contents is not necessary, because if opt_hostname != NULL, then the pointer in server_hostname is replaced (it is pre-cleared by kfree), respectively, everything will be cleared by itself with the current code. In smb3_fs_context_dup, opt_hostname does not need to be processed, since this variable is essentially temporary. Immediately after parsing with the parameter, its value goes to server_hostname and it is no longer needed by itself. > Thanks, > >> char *UNC; >> char *nodename; >> char workstation_name[CIFS_MAX_WORKSTATION_LEN]; >> >> base-commit: bec6f00f120ea68ba584def5b7416287e7dd29a7 >> -- >> 2.42.2 >> Apologies for the overly long subject line and unclear description. Thanks.