[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Sat, 8 Jun 2013 20:05:13 +0200
From: Dániel Bali <balijanosdaniel@...il.com>
To: john-dev@...ts.openwall.com
Subject: Re: sha3-opencl
Hello!
Thank you for your feedback!
2013/6/8 Lukas Odzioba <lukas.odzioba@...il.com>
>
> 1) current copyright:
> [...]
> According to this document:
> http://openwall.info/wiki/john/licensing
> I think that it would be better if it looks like this:
> [...]
>
I changed the copyright comments accordingly.
> 2) You should add a few more tests, for example an empty string and
> string with lenght = PLAINTEXT_LENGTH
>
I addressed this.
> 3) Please get rid of those warnings:
> opencl_rawkeccak256_fmt.c: In function ‘crypt_all’:
> opencl_rawkeccak256_fmt.c:331:9: warning: unused variable ‘i’
> [-Wunused-variable]
>
Sorry, that was from testing and I haven't cleaned up the code before
committing.
> 4) crypt_all_benchmark and crypt_all functions looks identical for me.
>
That was copied from opencl_rawmd5_fmt.c. I tried removing it and changing
all references to "crypt_all_benchmark" to "crypt_all" but it caused
errors. My guess is that the benchmark version is used for benchmarking
from the outside.
> 5) #define BUFSIZE ((PLAINTEXT_LENGTH+3)/4*4) //This is
> eqal to 3, are you sure this is a proper value?
>
I also copied this fromopencl_rawmd5_fmt.c. It is equal to 56 by the way.
My guess is that x/4*4 makes sure the value is divisible by 4, which might
be because of the 4 byte integers. I'm not sure.
> 6) I suppose that your problems with memory are related to mess in
> size of the buffers:
> One bug below:
> buffer_out = clCreateBuffer(context[ocl_gpu_id], CL_MEM_WRITE_ONLY,
> DIGEST_SIZE * kpc, NULL, &ret_code);
> #define CIPHERTEXT_LENGTH 64
> #define DIGEST_SIZE 32
This was also copied. The output should be 32 bytes for each hash, so why
would this be an error?
Claudio André pointed out the error that caused the segfaults/double free
errors. It was this line:
res_hashes = malloc(sizeof(cl_uint) * 3 * kpc);
I changed the 3 to 7 and now the kernel runs on both devices with any
MAX_KEYS_PER_CRYPT value, so I guess it works now.
I'm not sure how could I miss this line because I was looking for constants
like this that needed to be changed because of the hash size difference
between keccak256 and md5. Maybe because I was focused on the value 4 much
more.
Thanks again for all the help and sorry for being slow at tmes.
I'll try to clean up the code and commit.
Regards,
Daniel
Content of type "text/html" skipped
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
