All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 4/8] ntlmv2/ntlmssp  define, declare, and use crypto hash functions
@ 2010-09-08  4:45 shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w
       [not found] ` <1283921151-13090-1-git-send-email-shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  0 siblings, 1 reply; 5+ messages in thread
From: shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w @ 2010-09-08  4:45 UTC (permalink / raw)
  To: smfrench-Re5JQEeQqe8AvxtiuMwx3w
  Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA, Shirish Pargaonkar

From: Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>


Allocate crypto hashing functions, ecurity descriptiors, and respective
contexts when a smb/tcp connection is established.
Release them when a tcp/smb connection is taken down.

md5 and hmac-md5 are two crypto hashing functions that are used
throught the life of an smb/tcp connection by various functions that
calcualte signagure and ntlmv2 hash, HMAC etc.


Signed-off-by: Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 fs/cifs/cifsencrypt.c |   71 +++++++++++++++++++++++++++++++++++++++++++++++++
 fs/cifs/cifsproto.h   |    2 +
 fs/cifs/connect.c     |   16 +++++++++--
 3 files changed, 86 insertions(+), 3 deletions(-)

diff --git a/fs/cifs/cifsencrypt.c b/fs/cifs/cifsencrypt.c
index 4bdcf13..4772c4d 100644
--- a/fs/cifs/cifsencrypt.c
+++ b/fs/cifs/cifsencrypt.c
@@ -369,3 +369,74 @@ void CalcNTLMv2_response(const struct cifsSesInfo *ses,
 	hmac_md5_final(v2_session_response, &context);
 /*	cifs_dump_mem("v2_sess_rsp: ", v2_session_response, 32); */
 }
+
+void
+cifs_crypto_shash_release(struct TCP_Server_Info *server)
+{
+	if (server->secmech.md5)
+		crypto_free_shash(server->secmech.md5);
+
+	if (server->secmech.hmacmd5)
+		crypto_free_shash(server->secmech.hmacmd5);
+
+	kfree(server->secmech.sdeschmacmd5);
+
+	kfree(server->secmech.sdescmd5);
+}
+
+int
+cifs_crypto_shash_allocate(struct TCP_Server_Info *server)
+{
+	int rc;
+	unsigned int size;
+
+	server->secmech.hmacmd5 = crypto_alloc_shash("hmac(md5)", 0, 0);
+	if (!server->secmech.hmacmd5 ||
+			IS_ERR(server->secmech.hmacmd5)) {
+		cERROR(1, "could not allocate crypto hmacmd5\n");
+		return 1;
+	}
+
+	server->secmech.md5 = crypto_alloc_shash("md5", 0, 0);
+	if (!server->secmech.md5 || IS_ERR(server->secmech.md5)) {
+		cERROR(1, "could not allocate crypto md5\n");
+		rc = 1;
+		goto cifs_crypto_shash_allocate_ret1;
+	}
+
+	size = sizeof(struct shash_desc) +
+			crypto_shash_descsize(server->secmech.hmacmd5);
+	server->secmech.sdeschmacmd5 = kmalloc(size, GFP_KERNEL);
+	if (!server->secmech.sdeschmacmd5) {
+		cERROR(1, "cifs_crypto_shash_allocate: can't alloc hmacmd5\n");
+		rc = -ENOMEM;
+		goto cifs_crypto_shash_allocate_ret2;
+	}
+	server->secmech.sdeschmacmd5->shash.tfm = server->secmech.hmacmd5;
+	server->secmech.sdeschmacmd5->shash.flags = 0x0;
+
+
+	size = sizeof(struct shash_desc) +
+			crypto_shash_descsize(server->secmech.md5);
+	server->secmech.sdescmd5 = kmalloc(size, GFP_KERNEL);
+	if (!server->secmech.sdescmd5) {
+		cERROR(1, "cifs_crypto_shash_allocate: can't alloc md5\n");
+		rc = -ENOMEM;
+		goto cifs_crypto_shash_allocate_ret3;
+	}
+	server->secmech.sdescmd5->shash.tfm = server->secmech.md5;
+	server->secmech.sdescmd5->shash.flags = 0x0;
+
+	return 0;
+
+cifs_crypto_shash_allocate_ret3:
+	kfree(server->secmech.sdeschmacmd5);
+
+cifs_crypto_shash_allocate_ret2:
+	crypto_free_shash(server->secmech.md5);
+
+cifs_crypto_shash_allocate_ret1:
+	crypto_free_shash(server->secmech.hmacmd5);
+
+	return rc;
+}
diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index eadf78c..fa3716c 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -368,6 +368,8 @@ extern int cifs_calculate_mac_key(struct session_key *key, const char *rn,
 extern void CalcNTLMv2_response(const struct cifsSesInfo *, char *);
 extern void setup_ntlmv2_rsp(struct cifsSesInfo *, char *,
 			     const struct nls_table *);
+extern int cifs_crypto_shash_allocate(struct TCP_Server_Info *);
+extern void cifs_crypto_shash_release(struct TCP_Server_Info *);
 #ifdef CONFIG_CIFS_WEAK_PW_HASH
 extern void calc_lanman_hash(const char *password, const char *cryptkey,
 				bool encrypt, char *lnm_session_key);
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 0ea52e9..f5369e7 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -1520,6 +1520,7 @@ cifs_put_tcp_session(struct TCP_Server_Info *server)
 	server->tcpStatus = CifsExiting;
 	spin_unlock(&GlobalMid_Lock);
 
+	cifs_crypto_shash_release(server);
 	cifs_fscache_release_client_cookie(server);
 
 	task = xchg(&server->tsk, NULL);
@@ -1574,10 +1575,16 @@ cifs_get_tcp_session(struct smb_vol *volume_info)
 		goto out_err;
 	}
 
+	rc = cifs_crypto_shash_allocate(tcp_ses);
+	if (rc) {
+		cERROR(1, "could not setup hash structures rc %d", rc);
+		goto out_err;
+	}
+
 	tcp_ses->hostname = extract_hostname(volume_info->UNC);
 	if (IS_ERR(tcp_ses->hostname)) {
 		rc = PTR_ERR(tcp_ses->hostname);
-		goto out_err;
+		goto out_err2;
 	}
 
 	tcp_ses->noblocksnd = volume_info->noblocksnd;
@@ -1618,7 +1625,7 @@ cifs_get_tcp_session(struct smb_vol *volume_info)
 	}
 	if (rc < 0) {
 		cERROR(1, "Error connecting to socket. Aborting operation");
-		goto out_err;
+		goto out_err2;
 	}
 
 	/*
@@ -1632,7 +1639,7 @@ cifs_get_tcp_session(struct smb_vol *volume_info)
 		rc = PTR_ERR(tcp_ses->tsk);
 		cERROR(1, "error %d create cifsd thread", rc);
 		module_put(THIS_MODULE);
-		goto out_err;
+		goto out_err2;
 	}
 
 	/* thread spawned, put it on the list */
@@ -1644,6 +1651,9 @@ cifs_get_tcp_session(struct smb_vol *volume_info)
 
 	return tcp_ses;
 
+out_err2:
+	cifs_crypto_shash_release(tcp_ses);
+
 out_err:
 	if (tcp_ses) {
 		if (!IS_ERR(tcp_ses->hostname))
-- 
1.6.0.2

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 4/8] ntlmv2/ntlmssp  define, declare, and use crypto hash functions
       [not found] ` <1283921151-13090-1-git-send-email-shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2010-09-08 20:00   ` Jeff Layton
  2010-09-09 12:00   ` Suresh Jayaraman
  1 sibling, 0 replies; 5+ messages in thread
From: Jeff Layton @ 2010-09-08 20:00 UTC (permalink / raw)
  To: shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w
  Cc: smfrench-Re5JQEeQqe8AvxtiuMwx3w,
	linux-cifs-u79uwXL29TY76Z2rM5mHXA

On Tue,  7 Sep 2010 23:45:51 -0500
shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:

> From: Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> 
> 
> Allocate crypto hashing functions, ecurity descriptiors, and respective
> contexts when a smb/tcp connection is established.
> Release them when a tcp/smb connection is taken down.
> 
> md5 and hmac-md5 are two crypto hashing functions that are used
> throught the life of an smb/tcp connection by various functions that
> calcualte signagure and ntlmv2 hash, HMAC etc.
> 
> 
> Signed-off-by: Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  fs/cifs/cifsencrypt.c |   71 +++++++++++++++++++++++++++++++++++++++++++++++++
>  fs/cifs/cifsproto.h   |    2 +
>  fs/cifs/connect.c     |   16 +++++++++--
>  3 files changed, 86 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/cifs/cifsencrypt.c b/fs/cifs/cifsencrypt.c
> index 4bdcf13..4772c4d 100644
> --- a/fs/cifs/cifsencrypt.c
> +++ b/fs/cifs/cifsencrypt.c
> @@ -369,3 +369,74 @@ void CalcNTLMv2_response(const struct cifsSesInfo *ses,
>  	hmac_md5_final(v2_session_response, &context);
>  /*	cifs_dump_mem("v2_sess_rsp: ", v2_session_response, 32); */
>  }
> +
> +void
> +cifs_crypto_shash_release(struct TCP_Server_Info *server)
> +{
> +	if (server->secmech.md5)
> +		crypto_free_shash(server->secmech.md5);
> +
> +	if (server->secmech.hmacmd5)
> +		crypto_free_shash(server->secmech.hmacmd5);
> +
> +	kfree(server->secmech.sdeschmacmd5);
> +
> +	kfree(server->secmech.sdescmd5);
> +}
> +
> +int
> +cifs_crypto_shash_allocate(struct TCP_Server_Info *server)
> +{
> +	int rc;
> +	unsigned int size;
> +
> +	server->secmech.hmacmd5 = crypto_alloc_shash("hmac(md5)", 0, 0);
> +	if (!server->secmech.hmacmd5 ||
> +			IS_ERR(server->secmech.hmacmd5)) {
> +		cERROR(1, "could not allocate crypto hmacmd5\n");
> +		return 1;
> +	}
> +
> +	server->secmech.md5 = crypto_alloc_shash("md5", 0, 0);
> +	if (!server->secmech.md5 || IS_ERR(server->secmech.md5)) {
> +		cERROR(1, "could not allocate crypto md5\n");
> +		rc = 1;
> +		goto cifs_crypto_shash_allocate_ret1;
> +	}
> +
> +	size = sizeof(struct shash_desc) +
> +			crypto_shash_descsize(server->secmech.hmacmd5);
> +	server->secmech.sdeschmacmd5 = kmalloc(size, GFP_KERNEL);
> +	if (!server->secmech.sdeschmacmd5) {
> +		cERROR(1, "cifs_crypto_shash_allocate: can't alloc hmacmd5\n");
> +		rc = -ENOMEM;
> +		goto cifs_crypto_shash_allocate_ret2;
> +	}
> +	server->secmech.sdeschmacmd5->shash.tfm = server->secmech.hmacmd5;
> +	server->secmech.sdeschmacmd5->shash.flags = 0x0;
> +
> +
> +	size = sizeof(struct shash_desc) +
> +			crypto_shash_descsize(server->secmech.md5);
> +	server->secmech.sdescmd5 = kmalloc(size, GFP_KERNEL);
> +	if (!server->secmech.sdescmd5) {
> +		cERROR(1, "cifs_crypto_shash_allocate: can't alloc md5\n");
> +		rc = -ENOMEM;
> +		goto cifs_crypto_shash_allocate_ret3;
> +	}
> +	server->secmech.sdescmd5->shash.tfm = server->secmech.md5;
> +	server->secmech.sdescmd5->shash.flags = 0x0;
> +
> +	return 0;
> +
> +cifs_crypto_shash_allocate_ret3:
> +	kfree(server->secmech.sdeschmacmd5);
> +
> +cifs_crypto_shash_allocate_ret2:
> +	crypto_free_shash(server->secmech.md5);
> +
> +cifs_crypto_shash_allocate_ret1:
> +	crypto_free_shash(server->secmech.hmacmd5);
> +
> +	return rc;
> +}
> diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
> index eadf78c..fa3716c 100644
> --- a/fs/cifs/cifsproto.h
> +++ b/fs/cifs/cifsproto.h
> @@ -368,6 +368,8 @@ extern int cifs_calculate_mac_key(struct session_key *key, const char *rn,
>  extern void CalcNTLMv2_response(const struct cifsSesInfo *, char *);
>  extern void setup_ntlmv2_rsp(struct cifsSesInfo *, char *,
>  			     const struct nls_table *);
> +extern int cifs_crypto_shash_allocate(struct TCP_Server_Info *);
> +extern void cifs_crypto_shash_release(struct TCP_Server_Info *);
>  #ifdef CONFIG_CIFS_WEAK_PW_HASH
>  extern void calc_lanman_hash(const char *password, const char *cryptkey,
>  				bool encrypt, char *lnm_session_key);
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 0ea52e9..f5369e7 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -1520,6 +1520,7 @@ cifs_put_tcp_session(struct TCP_Server_Info *server)
>  	server->tcpStatus = CifsExiting;
>  	spin_unlock(&GlobalMid_Lock);
>  
> +	cifs_crypto_shash_release(server);
>  	cifs_fscache_release_client_cookie(server);
>  
>  	task = xchg(&server->tsk, NULL);
> @@ -1574,10 +1575,16 @@ cifs_get_tcp_session(struct smb_vol *volume_info)
>  		goto out_err;
>  	}
>  
> +	rc = cifs_crypto_shash_allocate(tcp_ses);
> +	if (rc) {
> +		cERROR(1, "could not setup hash structures rc %d", rc);
> +		goto out_err;
> +	}
> +
>  	tcp_ses->hostname = extract_hostname(volume_info->UNC);
>  	if (IS_ERR(tcp_ses->hostname)) {
>  		rc = PTR_ERR(tcp_ses->hostname);
> -		goto out_err;
> +		goto out_err2;
>  	}
>  
>  	tcp_ses->noblocksnd = volume_info->noblocksnd;
> @@ -1618,7 +1625,7 @@ cifs_get_tcp_session(struct smb_vol *volume_info)
>  	}
>  	if (rc < 0) {
>  		cERROR(1, "Error connecting to socket. Aborting operation");
> -		goto out_err;
> +		goto out_err2;
>  	}
>  
>  	/*
> @@ -1632,7 +1639,7 @@ cifs_get_tcp_session(struct smb_vol *volume_info)
>  		rc = PTR_ERR(tcp_ses->tsk);
>  		cERROR(1, "error %d create cifsd thread", rc);
>  		module_put(THIS_MODULE);
> -		goto out_err;
> +		goto out_err2;
>  	}
>  
>  	/* thread spawned, put it on the list */
> @@ -1644,6 +1651,9 @@ cifs_get_tcp_session(struct smb_vol *volume_info)
>  
>  	return tcp_ses;
>  
> +out_err2:
> +	cifs_crypto_shash_release(tcp_ses);
> +
>  out_err:
>  	if (tcp_ses) {
>  		if (!IS_ERR(tcp_ses->hostname))

Looks reasonable.

Acked-by: Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 4/8] ntlmv2/ntlmssp  define, declare, and use crypto hash functions
       [not found] ` <1283921151-13090-1-git-send-email-shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  2010-09-08 20:00   ` Jeff Layton
@ 2010-09-09 12:00   ` Suresh Jayaraman
  2010-09-09 16:13     ` Shirish Pargaonkar
  1 sibling, 1 reply; 5+ messages in thread
From: Suresh Jayaraman @ 2010-09-09 12:00 UTC (permalink / raw)
  To: shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w
  Cc: smfrench-Re5JQEeQqe8AvxtiuMwx3w,
	linux-cifs-u79uwXL29TY76Z2rM5mHXA

On 09/08/2010 10:15 AM, shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
> From: Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> 
> 
> Allocate crypto hashing functions, ecurity descriptiors, and respective
> contexts when a smb/tcp connection is established.
> Release them when a tcp/smb connection is taken down.
> 
> md5 and hmac-md5 are two crypto hashing functions that are used
> throught the life of an smb/tcp connection by various functions that
> calcualte signagure and ntlmv2 hash, HMAC etc.
> 
> 
> Signed-off-by: Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  fs/cifs/cifsencrypt.c |   71 +++++++++++++++++++++++++++++++++++++++++++++++++
>  fs/cifs/cifsproto.h   |    2 +
>  fs/cifs/connect.c     |   16 +++++++++--
>  3 files changed, 86 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/cifs/cifsencrypt.c b/fs/cifs/cifsencrypt.c
> index 4bdcf13..4772c4d 100644
> --- a/fs/cifs/cifsencrypt.c
> +++ b/fs/cifs/cifsencrypt.c
> @@ -369,3 +369,74 @@ void CalcNTLMv2_response(const struct cifsSesInfo *ses,
>  	hmac_md5_final(v2_session_response, &context);
>  /*	cifs_dump_mem("v2_sess_rsp: ", v2_session_response, 32); */
>  }
> +
> +void
> +cifs_crypto_shash_release(struct TCP_Server_Info *server)
> +{
> +	if (server->secmech.md5)
> +		crypto_free_shash(server->secmech.md5);
> +
> +	if (server->secmech.hmacmd5)
> +		crypto_free_shash(server->secmech.hmacmd5);
> +
> +	kfree(server->secmech.sdeschmacmd5);
> +
> +	kfree(server->secmech.sdescmd5);
> +}
> +
> +int
> +cifs_crypto_shash_allocate(struct TCP_Server_Info *server)
> +{
> +	int rc;
> +	unsigned int size;
> +
> +	server->secmech.hmacmd5 = crypto_alloc_shash("hmac(md5)", 0, 0);
> +	if (!server->secmech.hmacmd5 ||
> +			IS_ERR(server->secmech.hmacmd5)) {

crypto_alloc_hash() seems to return a pointer to struct crypto_shash.
Would it be sufficient to use IS_ERR() to check?

Also, instead of returning 1, we should use PTR_ERR() to propagate the
appropriate error back. Otherwise, rc will always be 1 which gives
little clue in case of error..


> +		cERROR(1, "could not allocate crypto hmacmd5\n");
> +		return 1;
> +	}
> +
> +	server->secmech.md5 = crypto_alloc_shash("md5", 0, 0);
> +	if (!server->secmech.md5 || IS_ERR(server->secmech.md5)) {
> +		cERROR(1, "could not allocate crypto md5\n");
> +		rc = 1;

ditto here..

> +		goto cifs_crypto_shash_allocate_ret1;

nit: the goto labels could use better names..?

> +	}

> +
> +	size = sizeof(struct shash_desc) +
> +			crypto_shash_descsize(server->secmech.hmacmd5);
> +	server->secmech.sdeschmacmd5 = kmalloc(size, GFP_KERNEL);
> +	if (!server->secmech.sdeschmacmd5) {
> +		cERROR(1, "cifs_crypto_shash_allocate: can't alloc hmacmd5\n");
> +		rc = -ENOMEM;
> +		goto cifs_crypto_shash_allocate_ret2;
> +	}
> +	server->secmech.sdeschmacmd5->shash.tfm = server->secmech.hmacmd5;
> +	server->secmech.sdeschmacmd5->shash.flags = 0x0;
> +
> +
> +	size = sizeof(struct shash_desc) +
> +			crypto_shash_descsize(server->secmech.md5);
> +	server->secmech.sdescmd5 = kmalloc(size, GFP_KERNEL);
> +	if (!server->secmech.sdescmd5) {
> +		cERROR(1, "cifs_crypto_shash_allocate: can't alloc md5\n");
> +		rc = -ENOMEM;
> +		goto cifs_crypto_shash_allocate_ret3;
> +	}
> +	server->secmech.sdescmd5->shash.tfm = server->secmech.md5;
> +	server->secmech.sdescmd5->shash.flags = 0x0;
> +
> +	return 0;
> +
> +cifs_crypto_shash_allocate_ret3:
> +	kfree(server->secmech.sdeschmacmd5);
> +
> +cifs_crypto_shash_allocate_ret2:
> +	crypto_free_shash(server->secmech.md5);
> +
> +cifs_crypto_shash_allocate_ret1:
> +	crypto_free_shash(server->secmech.hmacmd5);
> +
> +	return rc;
> +}
> diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
> index eadf78c..fa3716c 100644
> --- a/fs/cifs/cifsproto.h
> +++ b/fs/cifs/cifsproto.h
> @@ -368,6 +368,8 @@ extern int cifs_calculate_mac_key(struct session_key *key, const char *rn,
>  extern void CalcNTLMv2_response(const struct cifsSesInfo *, char *);
>  extern void setup_ntlmv2_rsp(struct cifsSesInfo *, char *,
>  			     const struct nls_table *);
> +extern int cifs_crypto_shash_allocate(struct TCP_Server_Info *);
> +extern void cifs_crypto_shash_release(struct TCP_Server_Info *);
>  #ifdef CONFIG_CIFS_WEAK_PW_HASH
>  extern void calc_lanman_hash(const char *password, const char *cryptkey,
>  				bool encrypt, char *lnm_session_key);
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 0ea52e9..f5369e7 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -1520,6 +1520,7 @@ cifs_put_tcp_session(struct TCP_Server_Info *server)
>  	server->tcpStatus = CifsExiting;
>  	spin_unlock(&GlobalMid_Lock);
>  
> +	cifs_crypto_shash_release(server);
>  	cifs_fscache_release_client_cookie(server);
>  
>  	task = xchg(&server->tsk, NULL);
> @@ -1574,10 +1575,16 @@ cifs_get_tcp_session(struct smb_vol *volume_info)
>  		goto out_err;
>  	}
>  
> +	rc = cifs_crypto_shash_allocate(tcp_ses);
> +	if (rc) {
> +		cERROR(1, "could not setup hash structures rc %d", rc);
> +		goto out_err;
> +	}
> +
>  	tcp_ses->hostname = extract_hostname(volume_info->UNC);
>  	if (IS_ERR(tcp_ses->hostname)) {
>  		rc = PTR_ERR(tcp_ses->hostname);
> -		goto out_err;
> +		goto out_err2;
>  	}
>  
>  	tcp_ses->noblocksnd = volume_info->noblocksnd;
> @@ -1618,7 +1625,7 @@ cifs_get_tcp_session(struct smb_vol *volume_info)
>  	}
>  	if (rc < 0) {
>  		cERROR(1, "Error connecting to socket. Aborting operation");
> -		goto out_err;
> +		goto out_err2;
>  	}
>  
>  	/*
> @@ -1632,7 +1639,7 @@ cifs_get_tcp_session(struct smb_vol *volume_info)
>  		rc = PTR_ERR(tcp_ses->tsk);
>  		cERROR(1, "error %d create cifsd thread", rc);
>  		module_put(THIS_MODULE);
> -		goto out_err;
> +		goto out_err2;
>  	}
>  
>  	/* thread spawned, put it on the list */
> @@ -1644,6 +1651,9 @@ cifs_get_tcp_session(struct smb_vol *volume_info)
>  
>  	return tcp_ses;
>  
> +out_err2:
> +	cifs_crypto_shash_release(tcp_ses);
> +
>  out_err:
>  	if (tcp_ses) {
>  		if (!IS_ERR(tcp_ses->hostname))


-- 
Suresh Jayaraman

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 4/8] ntlmv2/ntlmssp define, declare, and use crypto hash functions
  2010-09-09 12:00   ` Suresh Jayaraman
@ 2010-09-09 16:13     ` Shirish Pargaonkar
       [not found]       ` <AANLkTinAA+5aXp0mO=h4f3TiLrL6PR4Uu_DHw2Ched3J-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 5+ messages in thread
From: Shirish Pargaonkar @ 2010-09-09 16:13 UTC (permalink / raw)
  To: Suresh Jayaraman; +Cc: smfrench, linux-cifs, linux-crypto

On Thu, Sep 9, 2010 at 7:00 AM, Suresh Jayaraman <sjayaraman@suse.de> wrote:
> On 09/08/2010 10:15 AM, shirishpargaonkar@gmail.com wrote:
>> From: Shirish Pargaonkar <shirishpargaonkar@gmail.com>
>>
>>
>> Allocate crypto hashing functions, ecurity descriptiors, and respective
>> contexts when a smb/tcp connection is established.
>> Release them when a tcp/smb connection is taken down.
>>
>> md5 and hmac-md5 are two crypto hashing functions that are used
>> throught the life of an smb/tcp connection by various functions that
>> calcualte signagure and ntlmv2 hash, HMAC etc.
>>
>>
>> Signed-off-by: Shirish Pargaonkar <shirishpargaonkar@gmail.com>
>> ---
>>  fs/cifs/cifsencrypt.c |   71 +++++++++++++++++++++++++++++++++++++++++++++++++
>>  fs/cifs/cifsproto.h   |    2 +
>>  fs/cifs/connect.c     |   16 +++++++++--
>>  3 files changed, 86 insertions(+), 3 deletions(-)
>>
>> diff --git a/fs/cifs/cifsencrypt.c b/fs/cifs/cifsencrypt.c
>> index 4bdcf13..4772c4d 100644
>> --- a/fs/cifs/cifsencrypt.c
>> +++ b/fs/cifs/cifsencrypt.c
>> @@ -369,3 +369,74 @@ void CalcNTLMv2_response(const struct cifsSesInfo *ses,
>>       hmac_md5_final(v2_session_response, &context);
>>  /*   cifs_dump_mem("v2_sess_rsp: ", v2_session_response, 32); */
>>  }
>> +
>> +void
>> +cifs_crypto_shash_release(struct TCP_Server_Info *server)
>> +{
>> +     if (server->secmech.md5)
>> +             crypto_free_shash(server->secmech.md5);
>> +
>> +     if (server->secmech.hmacmd5)
>> +             crypto_free_shash(server->secmech.hmacmd5);
>> +
>> +     kfree(server->secmech.sdeschmacmd5);
>> +
>> +     kfree(server->secmech.sdescmd5);
>> +}
>> +
>> +int
>> +cifs_crypto_shash_allocate(struct TCP_Server_Info *server)
>> +{
>> +     int rc;
>> +     unsigned int size;
>> +
>> +     server->secmech.hmacmd5 = crypto_alloc_shash("hmac(md5)", 0, 0);
>> +     if (!server->secmech.hmacmd5 ||
>> +                     IS_ERR(server->secmech.hmacmd5)) {
>
> crypto_alloc_hash() seems to return a pointer to struct crypto_shash.
> Would it be sufficient to use IS_ERR() to check?

Suresh, not sure I understand, I check for NULL value of what
crypto_alloc_shash()
returns.  IS_ERR() is what crypto code is using.
Copying crypto folks on the this thread.

>
> Also, instead of returning 1, we should use PTR_ERR() to propagate the
> appropriate error back. Otherwise, rc will always be 1 which gives
> little clue in case of error..

Done.

>
>
>> +             cERROR(1, "could not allocate crypto hmacmd5\n");
>> +             return 1;
>> +     }
>> +
>> +     server->secmech.md5 = crypto_alloc_shash("md5", 0, 0);
>> +     if (!server->secmech.md5 || IS_ERR(server->secmech.md5)) {
>> +             cERROR(1, "could not allocate crypto md5\n");
>> +             rc = 1;
>
> ditto here..

Done here too.

>
>> +             goto cifs_crypto_shash_allocate_ret1;
>
> nit: the goto labels could use better names..?

OK, will make them more meaningful in the next iteration.

>
>> +     }
>
>> +
>> +     size = sizeof(struct shash_desc) +
>> +                     crypto_shash_descsize(server->secmech.hmacmd5);
>> +     server->secmech.sdeschmacmd5 = kmalloc(size, GFP_KERNEL);
>> +     if (!server->secmech.sdeschmacmd5) {
>> +             cERROR(1, "cifs_crypto_shash_allocate: can't alloc hmacmd5\n");
>> +             rc = -ENOMEM;
>> +             goto cifs_crypto_shash_allocate_ret2;
>> +     }
>> +     server->secmech.sdeschmacmd5->shash.tfm = server->secmech.hmacmd5;
>> +     server->secmech.sdeschmacmd5->shash.flags = 0x0;
>> +
>> +
>> +     size = sizeof(struct shash_desc) +
>> +                     crypto_shash_descsize(server->secmech.md5);
>> +     server->secmech.sdescmd5 = kmalloc(size, GFP_KERNEL);
>> +     if (!server->secmech.sdescmd5) {
>> +             cERROR(1, "cifs_crypto_shash_allocate: can't alloc md5\n");
>> +             rc = -ENOMEM;
>> +             goto cifs_crypto_shash_allocate_ret3;
>> +     }
>> +     server->secmech.sdescmd5->shash.tfm = server->secmech.md5;
>> +     server->secmech.sdescmd5->shash.flags = 0x0;
>> +
>> +     return 0;
>> +
>> +cifs_crypto_shash_allocate_ret3:
>> +     kfree(server->secmech.sdeschmacmd5);
>> +
>> +cifs_crypto_shash_allocate_ret2:
>> +     crypto_free_shash(server->secmech.md5);
>> +
>> +cifs_crypto_shash_allocate_ret1:
>> +     crypto_free_shash(server->secmech.hmacmd5);
>> +
>> +     return rc;
>> +}
>> diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
>> index eadf78c..fa3716c 100644
>> --- a/fs/cifs/cifsproto.h
>> +++ b/fs/cifs/cifsproto.h
>> @@ -368,6 +368,8 @@ extern int cifs_calculate_mac_key(struct session_key *key, const char *rn,
>>  extern void CalcNTLMv2_response(const struct cifsSesInfo *, char *);
>>  extern void setup_ntlmv2_rsp(struct cifsSesInfo *, char *,
>>                            const struct nls_table *);
>> +extern int cifs_crypto_shash_allocate(struct TCP_Server_Info *);
>> +extern void cifs_crypto_shash_release(struct TCP_Server_Info *);
>>  #ifdef CONFIG_CIFS_WEAK_PW_HASH
>>  extern void calc_lanman_hash(const char *password, const char *cryptkey,
>>                               bool encrypt, char *lnm_session_key);
>> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
>> index 0ea52e9..f5369e7 100644
>> --- a/fs/cifs/connect.c
>> +++ b/fs/cifs/connect.c
>> @@ -1520,6 +1520,7 @@ cifs_put_tcp_session(struct TCP_Server_Info *server)
>>       server->tcpStatus = CifsExiting;
>>       spin_unlock(&GlobalMid_Lock);
>>
>> +     cifs_crypto_shash_release(server);
>>       cifs_fscache_release_client_cookie(server);
>>
>>       task = xchg(&server->tsk, NULL);
>> @@ -1574,10 +1575,16 @@ cifs_get_tcp_session(struct smb_vol *volume_info)
>>               goto out_err;
>>       }
>>
>> +     rc = cifs_crypto_shash_allocate(tcp_ses);
>> +     if (rc) {
>> +             cERROR(1, "could not setup hash structures rc %d", rc);
>> +             goto out_err;
>> +     }
>> +
>>       tcp_ses->hostname = extract_hostname(volume_info->UNC);
>>       if (IS_ERR(tcp_ses->hostname)) {
>>               rc = PTR_ERR(tcp_ses->hostname);
>> -             goto out_err;
>> +             goto out_err2;
>>       }
>>
>>       tcp_ses->noblocksnd = volume_info->noblocksnd;
>> @@ -1618,7 +1625,7 @@ cifs_get_tcp_session(struct smb_vol *volume_info)
>>       }
>>       if (rc < 0) {
>>               cERROR(1, "Error connecting to socket. Aborting operation");
>> -             goto out_err;
>> +             goto out_err2;
>>       }
>>
>>       /*
>> @@ -1632,7 +1639,7 @@ cifs_get_tcp_session(struct smb_vol *volume_info)
>>               rc = PTR_ERR(tcp_ses->tsk);
>>               cERROR(1, "error %d create cifsd thread", rc);
>>               module_put(THIS_MODULE);
>> -             goto out_err;
>> +             goto out_err2;
>>       }
>>
>>       /* thread spawned, put it on the list */
>> @@ -1644,6 +1651,9 @@ cifs_get_tcp_session(struct smb_vol *volume_info)
>>
>>       return tcp_ses;
>>
>> +out_err2:
>> +     cifs_crypto_shash_release(tcp_ses);
>> +
>>  out_err:
>>       if (tcp_ses) {
>>               if (!IS_ERR(tcp_ses->hostname))
>
>
> --
> Suresh Jayaraman
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 4/8] ntlmv2/ntlmssp define, declare, and use crypto hash functions
       [not found]       ` <AANLkTinAA+5aXp0mO=h4f3TiLrL6PR4Uu_DHw2Ched3J-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2010-09-09 16:47         ` Suresh Jayaraman
  0 siblings, 0 replies; 5+ messages in thread
From: Suresh Jayaraman @ 2010-09-09 16:47 UTC (permalink / raw)
  To: Shirish Pargaonkar
  Cc: smfrench-Re5JQEeQqe8AvxtiuMwx3w,
	linux-cifs-u79uwXL29TY76Z2rM5mHXA,
	linux-crypto-u79uwXL29TY76Z2rM5mHXA

On 09/09/2010 09:43 PM, Shirish Pargaonkar wrote:
> On Thu, Sep 9, 2010 at 7:00 AM, Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org> wrote:
>> On 09/08/2010 10:15 AM, shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
>>> From: Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>
>>>
>>> Allocate crypto hashing functions, ecurity descriptiors, and respective
>>> contexts when a smb/tcp connection is established.
>>> Release them when a tcp/smb connection is taken down.
>>>
>>> md5 and hmac-md5 are two crypto hashing functions that are used
>>> throught the life of an smb/tcp connection by various functions that
>>> calcualte signagure and ntlmv2 hash, HMAC etc.
>>>
>>>
>>> Signed-off-by: Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>> ---
>>> �fs/cifs/cifsencrypt.c | � 71 +++++++++++++++++++++++++++++++++++++++++++++++++
>>> �fs/cifs/cifsproto.h � | � �2 +
>>> �fs/cifs/connect.c � � | � 16 +++++++++--
>>> �3 files changed, 86 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/fs/cifs/cifsencrypt.c b/fs/cifs/cifsencrypt.c
>>> index 4bdcf13..4772c4d 100644
>>> --- a/fs/cifs/cifsencrypt.c
>>> +++ b/fs/cifs/cifsencrypt.c
>>> @@ -369,3 +369,74 @@ void CalcNTLMv2_response(const struct cifsSesInfo *ses,
>>> � � � hmac_md5_final(v2_session_response, &context);
>>> �/* � cifs_dump_mem("v2_sess_rsp: ", v2_session_response, 32); */
>>> �}
>>> +
>>> +void
>>> +cifs_crypto_shash_release(struct TCP_Server_Info *server)
>>> +{
>>> + � � if (server->secmech.md5)
>>> + � � � � � � crypto_free_shash(server->secmech.md5);
>>> +
>>> + � � if (server->secmech.hmacmd5)
>>> + � � � � � � crypto_free_shash(server->secmech.hmacmd5);
>>> +
>>> + � � kfree(server->secmech.sdeschmacmd5);
>>> +
>>> + � � kfree(server->secmech.sdescmd5);
>>> +}
>>> +
>>> +int
>>> +cifs_crypto_shash_allocate(struct TCP_Server_Info *server)
>>> +{
>>> + � � int rc;
>>> + � � unsigned int size;
>>> +
>>> + � � server->secmech.hmacmd5 = crypto_alloc_shash("hmac(md5)", 0, 0);
>>> + � � if (!server->secmech.hmacmd5 ||
>>> + � � � � � � � � � � IS_ERR(server->secmech.hmacmd5)) {
>>
>> crypto_alloc_hash() seems to return a pointer to struct crypto_shash.
>> Would it be sufficient to use IS_ERR() to check?
> 
> Suresh, not sure I understand, I check for NULL value of what
> crypto_alloc_shash()
> returns.  IS_ERR() is what crypto code is using.
> Copying crypto folks on the this thread.

I was suggesting to remove the NULL value check as checking for
IS_ERR(server->secmech.hmacmd5 alone can handle NULL cases and is
sufficient.



-- 
Suresh Jayaraman

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2010-09-09 16:47 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-09-08  4:45 [PATCH 4/8] ntlmv2/ntlmssp define, declare, and use crypto hash functions shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w
     [not found] ` <1283921151-13090-1-git-send-email-shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2010-09-08 20:00   ` Jeff Layton
2010-09-09 12:00   ` Suresh Jayaraman
2010-09-09 16:13     ` Shirish Pargaonkar
     [not found]       ` <AANLkTinAA+5aXp0mO=h4f3TiLrL6PR4Uu_DHw2Ched3J-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2010-09-09 16:47         ` Suresh Jayaraman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.