Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix unaligned access to reference buffer during datatype conversion on 64-bit Solaris #206

Merged
merged 1 commit into from
Dec 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/H5Rint.c
Original file line number Diff line number Diff line change
Expand Up @@ -1041,12 +1041,13 @@ H5R__decode(const unsigned char *buf, size_t *nbytes, H5R_ref_priv_t *ref)

FUNC_ENTER_PACKAGE

/* Sanity checks */
HDassert(buf);
HDassert(nbytes);
HDassert(ref);
buf_size = *nbytes;

/* Don't decode if buffer size isn't big enough */
buf_size = *nbytes;
if (buf_size < H5R_ENCODE_HEADER_SIZE)
HGOTO_ERROR(H5E_REFERENCE, H5E_CANTDECODE, FAIL, "Buffer size is too small")

Expand Down Expand Up @@ -1107,6 +1108,7 @@ H5R__decode(const unsigned char *buf, size_t *nbytes, H5R_ref_priv_t *ref)
H5R_LOG_DEBUG("Decoded reference, filename=%s, obj_addr=%s, encode size=%u", ref->info.obj.filename,
H5R__print_token(ref->info.obj.token), ref->encode_size);

/* Set output info */
*nbytes = decode_size;

done:
Expand Down
17 changes: 11 additions & 6 deletions src/H5Tref.c
Original file line number Diff line number Diff line change
Expand Up @@ -589,6 +589,7 @@ H5T__ref_mem_write(H5VL_object_t *src_file, const void *src_buf, size_t src_size
H5F_t * src_f = NULL;
hid_t file_id = H5I_INVALID_HID;
H5R_ref_priv_t *dst_ref = (H5R_ref_priv_t *)dst_buf;
H5R_ref_priv_t tmp_ref; /* Temporary reference to decode into */
herr_t ret_value = SUCCEED;

FUNC_ENTER_STATIC
Expand All @@ -599,6 +600,7 @@ H5T__ref_mem_write(H5VL_object_t *src_file, const void *src_buf, size_t src_size
HDassert(src_size);
HDassert(dst_buf);
HDassert(dst_size == H5T_REF_MEM_SIZE);
HDcompile_assert(sizeof(*dst_ref) == sizeof(tmp_ref));

/* Memory-to-memory conversion to support vlen conversion */
if (NULL == src_file) {
Expand All @@ -624,21 +626,21 @@ H5T__ref_mem_write(H5VL_object_t *src_file, const void *src_buf, size_t src_size
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "invalid VOL object")

/* Make sure reference buffer is correctly initialized */
HDmemset(dst_buf, 0, dst_size);
HDmemset(&tmp_ref, 0, sizeof(tmp_ref));

switch (src_type) {
case H5R_OBJECT1: {
size_t token_size = H5F_SIZEOF_ADDR(src_f);

if (H5R__create_object((const H5O_token_t *)src_buf, token_size, dst_ref) < 0)
if (H5R__create_object((const H5O_token_t *)src_buf, token_size, &tmp_ref) < 0)
HGOTO_ERROR(H5E_REFERENCE, H5E_CANTCREATE, FAIL, "unable to create object reference")
} break;

case H5R_DATASET_REGION1: {
const struct H5Tref_dsetreg *src_reg = (const struct H5Tref_dsetreg *)src_buf;
size_t token_size = H5F_SIZEOF_ADDR(src_f);

if (H5R__create_region(&src_reg->token, token_size, src_reg->space, dst_ref) < 0)
if (H5R__create_region(&src_reg->token, token_size, src_reg->space, &tmp_ref) < 0)
HGOTO_ERROR(H5E_REFERENCE, H5E_CANTCREATE, FAIL, "unable to create region reference")

/* create_region creates its internal copy of the space */
Expand All @@ -655,7 +657,7 @@ H5T__ref_mem_write(H5VL_object_t *src_file, const void *src_buf, size_t src_size
case H5R_OBJECT2:
case H5R_ATTR:
/* Decode reference */
if (H5R__decode((const unsigned char *)src_buf, &src_size, dst_ref) < 0)
if (H5R__decode((const unsigned char *)src_buf, &src_size, &tmp_ref) < 0)
HGOTO_ERROR(H5E_REFERENCE, H5E_CANTDECODE, FAIL, "Cannot decode reference")
break;

Expand All @@ -667,17 +669,20 @@ H5T__ref_mem_write(H5VL_object_t *src_file, const void *src_buf, size_t src_size
} /* end switch */

/* If no filename set, this is not an external reference */
if (NULL == H5R_REF_FILENAME(dst_ref)) {
if (NULL == H5R_REF_FILENAME(&tmp_ref)) {
/* TODO temporary hack to retrieve file object */
if ((file_id = H5F_get_file_id(src_file, H5I_FILE, FALSE)) < 0)
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "not a file or file object")

/* Attach loc ID to reference and hold reference to it, this is a
* user exposed reference so set app_ref to TRUE. */
if (H5R__set_loc_id(dst_ref, file_id, TRUE, TRUE) < 0)
if (H5R__set_loc_id(&tmp_ref, file_id, TRUE, TRUE) < 0)
HGOTO_ERROR(H5E_REFERENCE, H5E_CANTSET, FAIL, "unable to attach location id to reference")
} /* end if */

/* Set output info */
HDmemcpy(dst_ref, &tmp_ref, sizeof(tmp_ref));

done:
if ((file_id != H5I_INVALID_HID) && (H5I_dec_ref(file_id) < 0))
HDONE_ERROR(H5E_REFERENCE, H5E_CANTDEC, FAIL, "unable to decrement refcount on location id")
Expand Down
67 changes: 32 additions & 35 deletions test/trefer.c
Original file line number Diff line number Diff line change
Expand Up @@ -566,7 +566,7 @@ test_reference_vlen_obj(void)
hsize_t vl_dims[] = {1};
hid_t dapl_id; /* Dataset access property list */
H5R_ref_t *wbuf, /* buffer to write to disk */
*rbuf; /* buffer read from disk */
*rbuf = NULL; /* buffer read from disk */
unsigned * ibuf, *obuf;
unsigned i, j; /* Counters */
H5O_type_t obj_type; /* Object type */
Expand All @@ -578,7 +578,6 @@ test_reference_vlen_obj(void)

/* Allocate write & read buffers */
wbuf = HDcalloc(sizeof(H5R_ref_t), SPACE1_DIM1);
rbuf = HDcalloc(sizeof(H5R_ref_t), SPACE1_DIM1);
ibuf = HDcalloc(sizeof(unsigned), SPACE1_DIM1);
obuf = HDcalloc(sizeof(unsigned), SPACE1_DIM1);

Expand Down Expand Up @@ -832,8 +831,6 @@ test_reference_cmpnd_obj(void)
hsize_t dims1[] = {SPACE1_DIM1};
hsize_t cmpnd_dims[] = {1};
hid_t dapl_id; /* Dataset access property list */
H5R_ref_t *wbuf, /* buffer to write to disk */
*rbuf; /* buffer read from disk */
unsigned * ibuf, *obuf;
unsigned i, j; /* Counters */
H5O_type_t obj_type; /* Object type */
Expand All @@ -844,8 +841,6 @@ test_reference_cmpnd_obj(void)
MESSAGE(5, ("Testing Object Reference Functions within compound type\n"));

/* Allocate write & read buffers */
wbuf = HDcalloc(sizeof(H5R_ref_t), SPACE1_DIM1);
rbuf = HDcalloc(sizeof(H5R_ref_t), SPACE1_DIM1);
ibuf = HDcalloc(sizeof(unsigned), SPACE1_DIM1);
obuf = HDcalloc(sizeof(unsigned), SPACE1_DIM1);

Expand Down Expand Up @@ -946,39 +941,38 @@ test_reference_cmpnd_obj(void)
dataset = H5Dcreate2(fid1, "Dataset3", tid1, sid1, H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT);
CHECK(dataset, H5I_INVALID_HID, "H5Dcreate2");

/* Reset buffer for writing */
HDmemset(&cmpnd_wbuf, 0, sizeof(cmpnd_wbuf));

/* Create reference to dataset */
ret = H5Rcreate_object(fid1, "/Group1/Dataset1", H5P_DEFAULT, &wbuf[0]);
ret = H5Rcreate_object(fid1, "/Group1/Dataset1", H5P_DEFAULT, &cmpnd_wbuf.ref0);
CHECK(ret, FAIL, "H5Rcreate_object");
ret = H5Rget_obj_type3(&wbuf[0], H5P_DEFAULT, &obj_type);
ret = H5Rget_obj_type3(&cmpnd_wbuf.ref0, H5P_DEFAULT, &obj_type);
CHECK(ret, FAIL, "H5Rget_obj_type3");
VERIFY(obj_type, H5O_TYPE_DATASET, "H5Rget_obj_type3");

/* Create reference to dataset */
ret = H5Rcreate_object(fid1, "/Group1/Dataset2", H5P_DEFAULT, &wbuf[1]);
ret = H5Rcreate_object(fid1, "/Group1/Dataset2", H5P_DEFAULT, &cmpnd_wbuf.ref1);
CHECK(ret, FAIL, "H5Rcreate_object");
ret = H5Rget_obj_type3(&wbuf[1], H5P_DEFAULT, &obj_type);
ret = H5Rget_obj_type3(&cmpnd_wbuf.ref1, H5P_DEFAULT, &obj_type);
CHECK(ret, FAIL, "H5Rget_obj_type3");
VERIFY(obj_type, H5O_TYPE_DATASET, "H5Rget_obj_type3");

/* Create reference to group */
ret = H5Rcreate_object(fid1, "/Group1", H5P_DEFAULT, &wbuf[2]);
ret = H5Rcreate_object(fid1, "/Group1", H5P_DEFAULT, &cmpnd_wbuf.ref2);
CHECK(ret, FAIL, "H5Rcreate_object");
ret = H5Rget_obj_type3(&wbuf[2], H5P_DEFAULT, &obj_type);
ret = H5Rget_obj_type3(&cmpnd_wbuf.ref2, H5P_DEFAULT, &obj_type);
CHECK(ret, FAIL, "H5Rget_obj_type3");
VERIFY(obj_type, H5O_TYPE_GROUP, "H5Rget_obj_type3");

/* Create reference to named datatype */
ret = H5Rcreate_object(fid1, "/Group1/Datatype1", H5P_DEFAULT, &wbuf[3]);
ret = H5Rcreate_object(fid1, "/Group1/Datatype1", H5P_DEFAULT, &cmpnd_wbuf.ref3);
CHECK(ret, FAIL, "H5Rcreate_object");
ret = H5Rget_obj_type3(&wbuf[3], H5P_DEFAULT, &obj_type);
ret = H5Rget_obj_type3(&cmpnd_wbuf.ref3, H5P_DEFAULT, &obj_type);
CHECK(ret, FAIL, "H5Rget_obj_type3");
VERIFY(obj_type, H5O_TYPE_NAMED_DATATYPE, "H5Rget_obj_type3");

/* Store dimensions */
cmpnd_wbuf.ref0 = wbuf[0];
cmpnd_wbuf.ref1 = wbuf[1];
cmpnd_wbuf.ref2 = wbuf[2];
cmpnd_wbuf.ref3 = wbuf[3];
cmpnd_wbuf.dim_idx = SPACE1_DIM1;

/* Write selection to disk */
Expand Down Expand Up @@ -1017,17 +1011,13 @@ test_reference_cmpnd_obj(void)
CHECK(ret, FAIL, "H5Dread");

VERIFY(cmpnd_rbuf.dim_idx, SPACE1_DIM1, "H5Dread");
rbuf[0] = cmpnd_rbuf.ref0;
rbuf[1] = cmpnd_rbuf.ref1;
rbuf[2] = cmpnd_rbuf.ref2;
rbuf[3] = cmpnd_rbuf.ref3;

/* Close datatype */
ret = H5Tclose(tid1);
CHECK(ret, FAIL, "H5Tclose");

/* Open dataset object */
dset2 = H5Ropen_object(&rbuf[0], H5P_DEFAULT, dapl_id);
dset2 = H5Ropen_object(&cmpnd_rbuf.ref0, H5P_DEFAULT, dapl_id);
CHECK(dset2, H5I_INVALID_HID, "H5Ropen_object");

/* Check information in referenced dataset */
Expand All @@ -1049,15 +1039,15 @@ test_reference_cmpnd_obj(void)
CHECK(ret, FAIL, "H5Dclose");

/* Open group object. GAPL isn't supported yet. But it's harmless to pass in */
group = H5Ropen_object(&rbuf[2], H5P_DEFAULT, H5P_DEFAULT);
group = H5Ropen_object(&cmpnd_rbuf.ref2, H5P_DEFAULT, H5P_DEFAULT);
CHECK(group, H5I_INVALID_HID, "H5Ropen_object");

/* Close group */
ret = H5Gclose(group);
CHECK(ret, FAIL, "H5Gclose");

/* Open datatype object. TAPL isn't supported yet. But it's harmless to pass in */
tid1 = H5Ropen_object(&rbuf[3], H5P_DEFAULT, H5P_DEFAULT);
tid1 = H5Ropen_object(&cmpnd_rbuf.ref3, H5P_DEFAULT, H5P_DEFAULT);
CHECK(tid1, H5I_INVALID_HID, "H5Ropen_object");

/* Verify correct datatype */
Expand Down Expand Up @@ -1088,18 +1078,25 @@ test_reference_cmpnd_obj(void)
CHECK(ret, FAIL, "H5Fclose");

/* Destroy references */
for (j = 0; j < SPACE1_DIM1; j++) {
ret = H5Rdestroy(&wbuf[j]);
CHECK(ret, FAIL, "H5Rdestroy");
}
for (j = 0; j < SPACE1_DIM1; j++) {
ret = H5Rdestroy(&rbuf[j]);
CHECK(ret, FAIL, "H5Rdestroy");
}
ret = H5Rdestroy(&cmpnd_wbuf.ref0);
CHECK(ret, FAIL, "H5Rdestroy");
ret = H5Rdestroy(&cmpnd_wbuf.ref1);
CHECK(ret, FAIL, "H5Rdestroy");
ret = H5Rdestroy(&cmpnd_wbuf.ref2);
CHECK(ret, FAIL, "H5Rdestroy");
ret = H5Rdestroy(&cmpnd_wbuf.ref3);
CHECK(ret, FAIL, "H5Rdestroy");

ret = H5Rdestroy(&cmpnd_rbuf.ref0);
CHECK(ret, FAIL, "H5Rdestroy");
ret = H5Rdestroy(&cmpnd_rbuf.ref1);
CHECK(ret, FAIL, "H5Rdestroy");
ret = H5Rdestroy(&cmpnd_rbuf.ref2);
CHECK(ret, FAIL, "H5Rdestroy");
ret = H5Rdestroy(&cmpnd_rbuf.ref3);
CHECK(ret, FAIL, "H5Rdestroy");

/* Free memory buffers */
HDfree(wbuf);
HDfree(rbuf);
HDfree(ibuf);
HDfree(obuf);
} /* test_reference_cmpnd_obj() */
Expand Down