2010-09-09 16:47:46

by Suresh Jayaraman

[permalink] [raw]
Subject: Re: [PATCH 4/8] ntlmv2/ntlmssp define, declare, and use crypto hash functions

On 09/09/2010 09:43 PM, Shirish Pargaonkar wrote:
> On Thu, Sep 9, 2010 at 7:00 AM, Suresh Jayaraman <[email protected]> wrote:
>> On 09/08/2010 10:15 AM, [email protected] wrote:
>>> From: Shirish Pargaonkar <[email protected]>
>>>
>>>
>>> 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 <[email protected]>
>>> ---
>>> �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