Re: [RFC PATCH 24/31] cifs: Convert SMB2 Negotiate Protocol request

[Enzo Matsumiya]

On 08/06, David Howells wrote:
> ...
>
> -static unsigned int
> -build_netname_ctxt(struct smb2_netname_neg_context *pneg_ctxt, char *hostname)
> +static size_t smb2_size_netname_ctxt(struct TCP_Server_Info *server)
> {
> +	size_t data_len;
> +
> +#if 0
> 	struct nls_table *cp = load_nls_default();
> +	const char *hostname;
>
> -	pneg_ctxt->ContextType = SMB2_NETNAME_NEGOTIATE_CONTEXT_ID;
> +	/* Only include up to first 100 bytes of server name in the NetName
> +	 * field.
> +	 */
> +	cifs_server_lock(pserver);
> +	hostname = pserver->hostname;
> +	if (hostname && hostname[0])
> +		data_len = cifs_size_strtoUTF16(hostname, 100, cp);
> +	cifs_server_unlock(pserver);
> +#else
> +	/* Now, we can't just measure the length of hostname as, unless we hold
> +	 * the lock, it may change under us, so allow maximum space for it.
> +	 */
> +	data_len = 400;
> +#endif
> +	return ALIGN8(sizeof(struct smb2_neg_context) + data_len);
> +}

Why was this commented out?  Your comment implies that you can't hold
the lock anymore there, but I couldn't find out why (with your patches
applied).

> -static void
> -assemble_neg_contexts(struct smb2_negotiate_req *req,
> -		      struct TCP_Server_Info *server, unsigned int *total_len)
> +static size_t smb2_size_neg_contexts(struct TCP_Server_Info *server,
> +				     size_t offset)
> {
> -	unsigned int ctxt_len, neg_context_count;
> 	struct TCP_Server_Info *pserver;
> -	char *pneg_ctxt;
> -	char *hostname;
> -
> -	if (*total_len > 200) {
> -		/* In case length corrupted don't want to overrun smb buffer */
> -		cifs_server_dbg(VFS, "Bad frame length assembling neg contexts\n");
> -		return;
> -	}
>
> 	/*
> 	 * round up total_len of fixed part of SMB3 negotiate request to 8
> 	 * byte boundary before adding negotiate contexts
> 	 */
> -	*total_len = ALIGN8(*total_len);
> +	offset = ALIGN8(offset);
> +	offset += ALIGN8(sizeof(struct smb2_preauth_neg_context));
> +	offset += ALIGN8(sizeof(struct smb2_encryption_neg_context));
>
> -	pneg_ctxt = (*total_len) + (char *)req;
> -	req->NegotiateContextOffset = cpu_to_le32(*total_len);
> +	/*
> +	 * secondary channels don't have the hostname field populated
> +	 * use the hostname field in the primary channel instead
> +	 */
> +	pserver = SERVER_IS_CHAN(server) ? server->primary_server : server;
> +	offset += smb2_size_netname_ctxt(pserver);

If you're keeping data_len=400 above, you could just drop
smb2_size_netname_ctxt() altogether and use
"ALIGN8(sizeof(struct smb2_neg_context) + 400)" directly here.