Skip to content

Commit

Permalink
Fix for HDFFV-11052: h5debug fails on a corrupted file (h5_nrefs_POC)… (
Browse files Browse the repository at this point in the history
HDFGroup#2291)

* Fix for HDFFV-11052: h5debug fails on a corrupted file (h5_nrefs_POC) producing a core dump.
When h5debug closes the corrupted file, the library calls H5F__dest() which performs all the
closing operations for the file "f" (H5F_t *) but just keeping note of errors in "ret_value"
all the way till the end of the routine.  The user-provided corrupted file has an illegal
file size causing failure when reading the image during the closing process.
At the end of this routine it sets f->shared to NULL and then frees "f".
This is done whether there is error or not in "ret_value".
Due to the failure in reading the file earlier, the routine then returns error.
The error return from H5F__dest() causes the file object "f" not being removed from the
ID node table.  When the library finally exits, it will try to close the
file objects in the table.  This causes assertion failure for f->file_id > 0.
Fix:
a) H5F_dest(): free the f only when there is no error in "ret_value" at the end of the routine.
b) H5F__close_cb(): if f->shared is NULL, free "f"; otherwise, perform closing on "f" as before.
c) h5debug.c main(): track error return from H5Fclose().

* Committing clang-format changes

Co-authored-by: vchoi <[email protected]>
Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
  • Loading branch information
3 people committed May 9, 2023
1 parent 811f5b5 commit 15116dc
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 27 deletions.
59 changes: 34 additions & 25 deletions src/H5Fint.c
Original file line number Diff line number Diff line change
Expand Up @@ -1466,7 +1466,9 @@ H5F__dest(H5F_t *f, hbool_t flush)
if (H5FO_top_dest(f) < 0)
HDONE_ERROR(H5E_FILE, H5E_CANTINIT, FAIL, "problems closing file")
f->shared = NULL;
f = H5FL_FREE(H5F_t, f);

if (ret_value >= 0)
f = H5FL_FREE(H5F_t, f);

FUNC_LEAVE_NOAPI(ret_value)
} /* end H5F__dest() */
Expand Down Expand Up @@ -2193,35 +2195,42 @@ H5F__close_cb(H5F_t *f)

/* Sanity check */
HDassert(f);
HDassert(f->file_id >
HDassert(f->shared == NULL || f->file_id >
0); /* This routine should only be called when a file ID's ref count drops to zero */

/* Perform checks for "semi" file close degree here, since closing the
* file is not allowed if there are objects still open.
*/
if (f->shared->fc_degree == H5F_CLOSE_SEMI) {
unsigned nopen_files = 0; /* Number of open files in file/mount hierarchy */
unsigned nopen_objs = 0; /* Number of open objects in file/mount hierarchy */

/* Get the number of open objects and open files on this file/mount hierarchy */
if (H5F__mount_count_ids(f, &nopen_files, &nopen_objs) < 0)
HGOTO_ERROR(H5E_SYM, H5E_MOUNT, FAIL, "problem checking mount hierarchy")

/* If there are no other file IDs open on this file/mount hier., but
* there are still open objects, issue an error and bail out now,
* without decrementing the file ID's reference count and triggering
* a "real" attempt at closing the file.
if (f->shared == NULL)
f = H5FL_FREE(H5F_t, f);

else {

/* Perform checks for "semi" file close degree here, since closing the
* file is not allowed if there are objects still open.
*/
if (nopen_files == 1 && nopen_objs > 0)
HGOTO_ERROR(H5E_FILE, H5E_CANTCLOSEFILE, FAIL, "can't close file, there are objects still open")
}
if (f->shared->fc_degree == H5F_CLOSE_SEMI) {
unsigned nopen_files = 0; /* Number of open files in file/mount hierarchy */
unsigned nopen_objs = 0; /* Number of open objects in file/mount hierarchy */

/* Get the number of open objects and open files on this file/mount hierarchy */
if (H5F__mount_count_ids(f, &nopen_files, &nopen_objs) < 0)
HGOTO_ERROR(H5E_SYM, H5E_MOUNT, FAIL, "problem checking mount hierarchy")

/* If there are no other file IDs open on this file/mount hier., but
* there are still open objects, issue an error and bail out now,
* without decrementing the file ID's reference count and triggering
* a "real" attempt at closing the file.
*/
if (nopen_files == 1 && nopen_objs > 0)
HGOTO_ERROR(H5E_FILE, H5E_CANTCLOSEFILE, FAIL, "can't close file, there are objects still open")
}

/* Reset the file ID for this file */
f->file_id = H5I_INVALID_HID;
/* Reset the file ID for this file */
f->file_id = H5I_INVALID_HID;

/* Attempt to close the file/mount hierarchy */
if (H5F_try_close(f, NULL) < 0)
HGOTO_ERROR(H5E_FILE, H5E_CANTCLOSEFILE, FAIL, "can't close file")

/* Attempt to close the file/mount hierarchy */
if (H5F_try_close(f, NULL) < 0)
HGOTO_ERROR(H5E_FILE, H5E_CANTCLOSEFILE, FAIL, "can't close file")
}

done:
FUNC_LEAVE_NOAPI(ret_value)
Expand Down
8 changes: 6 additions & 2 deletions tools/src/misc/h5debug.c
Original file line number Diff line number Diff line change
Expand Up @@ -808,8 +808,12 @@ main(int argc, char *argv[])
done:
if (fapl > 0)
H5Pclose(fapl);
if (fid > 0)
H5Fclose(fid);
if (fid > 0) {
if (H5Fclose(fid) < 0) {
HDfprintf(stderr, "Error in closing file!\n");
exit_value = 1;
}
}

/* Pop API context */
if (api_ctx_pushed)
Expand Down

0 comments on commit 15116dc

Please sign in to comment.