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.