Skip to content

Commit 590e63c

Browse files
committed
OpenCL formats: Fix many bugs in host code
Detected through testing with POCL in an ASan-enabled build. Fixes #5882
1 parent a3e6909 commit 590e63c

10 files changed

+24
-17
lines changed

src/formats.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -602,11 +602,11 @@ static char* is_key_right(struct fmt_main *format, int index,
602602
return err_buf;
603603
}
604604

605-
if (match == 0) {
605+
if (match <= 0) {
606606
if (options.verbosity > VERB_LEGACY)
607-
snprintf(err_buf, sizeof(err_buf), "crypt_all(%d) zero return %s", index + 1, ciphertext);
607+
snprintf(err_buf, sizeof(err_buf), "crypt_all(%d) = %d for %s", index + 1, match, ciphertext);
608608
else
609-
sprintf(err_buf, "crypt_all(%d) zero return", index + 1);
609+
sprintf(err_buf, "crypt_all(%d) = %d", index + 1, match);
610610
return err_buf;
611611
}
612612

src/formats.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -383,7 +383,8 @@ struct fmt_methods {
383383
* The count is passed by reference and must be updated by crypt_all() if it
384384
* computes other than the requested count (such as if it generates additional
385385
* candidate passwords on its own). The updated count is used for c/s rate
386-
* calculation. The return value is thus in the 0 to updated *count range. */
386+
* calculation. The return value is thus in the 0 to updated *count range.
387+
* As an exception, a -1 return is used on error during OpenCL auto-tune. */
387388
int (*crypt_all)(int *count, struct db_salt *salt);
388389

389390
/* These functions calculate a hash out of a ciphertext that has just been

src/krb5_tgs_common_plug.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ void *krb5tgs_get_salt(char *ciphertext)
158158

159159
/* Now build dynamic salt and return a pointer to a pointer to it */
160160
static krb5tgs_salt *psalt;
161-
psalt = mem_calloc(1, sizeof(krb5tgs_salt) + edata2len);
161+
psalt = mem_calloc(1, sizeof(krb5tgs_salt) + edata2len - 1);
162162
memcpy(psalt->edata1, edata1, 16);
163163
psalt->edata2len = edata2len;
164164

src/opencl_argon2_fmt_plug.c

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -983,20 +983,21 @@ static void set_salt(void *salt)
983983
static void kp_set_salt(void *salt)
984984
{
985985
keepass_salt = *((keepass_salt_t**)salt);
986+
size_t cur_salt_size = sizeof(keepass_salt_t) + keepass_salt->content_size - 1;
986987

987988
// The KeePass salt is dynamic size, but starts off with a plain
988989
// Argon2 salt after the dyna_salt_t header
989990
memcpy(&saved_salt, &keepass_salt->t_cost, sizeof(struct argon2_salt));
990991

991-
if (sizeof(keepass_salt_t) + keepass_salt->content_size - 1 > keepass_saltsize) {
992+
if (cur_salt_size > keepass_saltsize) {
992993
CLRELEASEBUFFER(cl_keepass_salt);
993-
keepass_saltsize = sizeof(keepass_salt_t) + keepass_salt->content_size - 1;
994+
keepass_saltsize = cur_salt_size;
994995
CLCREATEBUFFER(cl_keepass_salt, CL_RO, keepass_saltsize);
995996
CLKERNELARG(keepass_init, 1, cl_keepass_salt);
996997
//CLKERNELARG(keepass_argon2, 1, cl_keepass_salt);
997998
CLKERNELARG(keepass_final, 1, cl_keepass_salt);
998999
}
999-
CLWRITE(cl_keepass_salt, CL_FALSE, 0, keepass_saltsize, keepass_salt, NULL);
1000+
CLWRITE(cl_keepass_salt, CL_FALSE, 0, cur_salt_size, keepass_salt, NULL);
10001001
HANDLE_CLERROR(clFlush(queue[gpu_id]), "clFlush failed in keepass_set_salt()");
10011002
}
10021003

src/opencl_keepass_fmt_plug.c

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -265,13 +265,15 @@ static char *get_key(int index)
265265
return ret;
266266
}
267267

268+
/* The logic below is duplicated in opencl_argon2_fmt_plug.c: kp_set_salt() */
268269
static void set_salt(void *salt)
269270
{
270271
keepass_salt = *((keepass_salt_t**)salt);
272+
size_t cur_salt_size = sizeof(keepass_salt_t) + keepass_salt->content_size - 1;
271273

272-
if (sizeof(keepass_salt_t) + keepass_salt->content_size - 1 > saltsize) {
274+
if (cur_salt_size > saltsize) {
273275
CLRELEASEBUFFER(mem_salt);
274-
saltsize = sizeof(keepass_salt_t) + keepass_salt->content_size - 1;
276+
saltsize = cur_salt_size;
275277
CLCREATEBUFFER(mem_salt, CL_RO, saltsize);
276278
CLKERNELARG(kernel_init, 1, mem_salt);
277279
CLKERNELARG(kernel_loop_aes, 1, mem_salt);
@@ -281,7 +283,7 @@ static void set_salt(void *salt)
281283
#endif
282284
}
283285

284-
CLWRITE(mem_salt, CL_FALSE, 0, saltsize, keepass_salt, NULL);
286+
CLWRITE(mem_salt, CL_FALSE, 0, cur_salt_size, keepass_salt, NULL);
285287
CLWRITE(mem_autotune, CL_FALSE, 0, sizeof(ocl_autotune_running),
286288
&ocl_autotune_running, NULL);
287289
HANDLE_CLERROR(clFlush(queue[gpu_id]), "clFlush failed in set_salt()");

src/opencl_krb5_tgs_fmt_plug.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ static void create_clobj(size_t gws, struct fmt_main *self)
9999
intkeysize = sizeof(cl_uint) * gws;
100100
statesize = sizeof(krb5tgs_state) * gws * mask_int_cand.num_int_cand;
101101
outsize = sizeof(krb5tgs_out) * gws * mask_int_cand.num_int_cand;
102-
saltsize = sizeof(krb5tgs_salt) + krb5tgs_max_data_len;
102+
saltsize = sizeof(krb5tgs_salt) + krb5tgs_max_data_len - 1;
103103

104104
pinned_key = clCreateBuffer(context[gpu_id], CL_MEM_READ_ONLY | CL_MEM_HOST_WRITE_ONLY | CL_MEM_ALLOC_HOST_PTR, insize, NULL, &ret_code);
105105
if (ret_code == CL_SUCCESS)
@@ -284,12 +284,13 @@ static void reset(struct db_main *db)
284284
static void set_salt(void *salt)
285285
{
286286
cur_salt = *(krb5tgs_salt**)salt;
287+
size_t cur_salt_size = sizeof(krb5tgs_salt) + cur_salt->edata2len - 1;
287288

288289
if (cur_salt->edata2len > krb5tgs_max_data_len)
289290
error_msg("This should not happen, please report.");
290291

291292
HANDLE_CLERROR(clEnqueueWriteBuffer(queue[gpu_id], cl_salt, CL_FALSE, 0,
292-
saltsize, cur_salt, 0, NULL, NULL),
293+
cur_salt_size, cur_salt, 0, NULL, NULL),
293294
"Failed transferring salt");
294295
HANDLE_CLERROR(clFlush(queue[gpu_id]), "clFlush failed in set_salt()");
295296
}

src/opencl_o5logon_fmt_plug.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -298,7 +298,8 @@ static int crypt_all(int *pcount, struct db_salt *salt)
298298
for (int i = count; i <= gws; i++)
299299
key_idx[i] = key_buf_end;
300300

301-
CLWRITE_CRYPT(cl_key_buf, CL_FALSE, key_offset, key_buf_end - key_offset, key_buf + key_offset, multi_profilingEvent[0]);
301+
if (key_buf_end != key_offset)
302+
CLWRITE_CRYPT(cl_key_buf, CL_FALSE, key_offset, key_buf_end - key_offset, key_buf + key_offset, multi_profilingEvent[0]);
302303
CLWRITE_CRYPT(cl_key_idx, CL_FALSE, idx_offset, 4 * (gws + 1) - idx_offset, key_idx + (idx_offset / 4), multi_profilingEvent[1]);
303304

304305
if (!mask_gpu_is_static)

src/opencl_pdf_fmt_plug.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -348,7 +348,8 @@ static int crypt_all(int *pcount, struct db_salt *salt)
348348
for (int i = count; i <= gws; i++)
349349
saved_idx[i] = key_idx;
350350

351-
CLWRITE_CRYPT(cl_saved_key, CL_FALSE, key_offset, key_idx - key_offset, saved_key + key_offset, multi_profilingEvent[0]);
351+
if (key_idx != key_offset)
352+
CLWRITE_CRYPT(cl_saved_key, CL_FALSE, key_offset, key_idx - key_offset, saved_key + key_offset, multi_profilingEvent[0]);
352353
CLWRITE_CRYPT(cl_saved_idx, CL_FALSE, idx_offset, 4 * (gws + 1) - idx_offset, saved_idx + (idx_offset / 4), multi_profilingEvent[1]);
353354

354355
if (!mask_gpu_is_static)

src/opencl_rawmd4_fmt_plug.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -291,7 +291,7 @@ static char *split(char *ciphertext, int index, struct fmt_main *self)
291291

292292
static void *get_binary(char *ciphertext)
293293
{
294-
static uint32_t out[DIGEST_SIZE];
294+
static uint32_t out[DIGEST_SIZE / 4];
295295
char *p;
296296
int i;
297297
p = ciphertext + TAG_LENGTH;

src/opencl_rawmd5_fmt_plug.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -316,7 +316,7 @@ static char *split(char *ciphertext, int index, struct fmt_main *self)
316316

317317
static void *get_binary(char *ciphertext)
318318
{
319-
static uint32_t out[DIGEST_SIZE];
319+
static uint32_t out[DIGEST_SIZE / 4];
320320
char *p;
321321
int i;
322322
p = ciphertext + TAG_LENGTH;

0 commit comments

Comments
 (0)