Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJAsdNhrmK2495poZ+vsj+tZAccDXMKH=uYijRcsmA6a3Xt8OQ@mail.gmail.com>
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.