On Apr 20, 2023, at 3:07 PM, Vladimir 'phcoder' Serbinenko <phco...@gmail.com> wrote:
On Thu, Apr 20, 2023 at 8:00 PM Lidong Chen <lidong.c...@oracle.com<mailto:lidong.c...@oracle.com>> wrote: When an invalid node size is detected in grub_hfsplus_mount(), data pinter is freed. Thus, file->data is not set. The code should also set the grub error when that happens to indicate an error and to avoid accessing the unintialized file->data in grub_file_close(). file->data is set to NULL in grub_file_open as part of zalloc. What sets it to something else? Thanks for the review. Here is the flow that causes invalid access to file->data in grub_file_close(): static struct grub_hfsplus_data * grub_hfsplus_mount (grub_disk_t disk) { data->catalog_tree.nodesize = grub_be_to_cpu16 (header.nodesize); if (data->catalog_tree.nodesize < 2) goto fail; // grub_errno is not set to indicate the failure here. fail: grub_free (data); return 0; } static grub_err_t grub_hfsplus_open (struct grub_file *file, const char *name) { data = grub_hfsplus_mount (file->device->disk); if (!data) goto fail; fail: grub_free (data); return grub_errno; // return GRUB_ERR_NONE } grub_file_t grub_file_open (const char *name, enum grub_file_type type) { if ((file->fs->fs_open) (file, file_name) != GRUB_ERR_NONE) // grub_hfsplus_open() goto fail; file->name = grub_strdup (name); grub_errno = GRUB_ERR_NONE; return file; } static grub_err_t grub_hfsplus_close (grub_file_t file) { struct grub_hfsplus_data *data = (struct grub_hfsplus_data *) file->data; grub_free (data->opened_file.cbuf); // SIGSEGV here return GRUB_ERR_NONE; } Regards, Lidong Signed-off-by: Lidong Chen <lidong.c...@oracle.com> --- grub-core/fs/hfsplus.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/grub-core/fs/hfsplus.c b/grub-core/fs/hfsplus.c index 9c1f12574..cf13e8a63 100644 --- a/grub-core/fs/hfsplus.c +++ b/grub-core/fs/hfsplus.c @@ -357,7 +357,10 @@ grub_hfsplus_mount (grub_disk_t disk) (header.key_compare == GRUB_HFSPLUSX_BINARYCOMPARE)); if (data->catalog_tree.nodesize < 2) - goto fail; + { + grub_error (GRUB_ERR_BAD_FS, "invalid catalog node size"); + goto fail; + } if (grub_hfsplus_read_file (&data->extoverflow_tree.file, 0, 0, sizeof (struct grub_hfsplus_btnode), @@ -374,7 +377,10 @@ grub_hfsplus_mount (grub_disk_t disk) data->extoverflow_tree.nodesize = grub_be_to_cpu16 (header.nodesize); if (data->extoverflow_tree.nodesize < 2) - goto fail; + { + grub_error (GRUB_ERR_BAD_FS, "invalid extents overflow node size"); + goto fail; + } data->extoverflow_tree_ready = 1; -- 2.39.1 _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org<mailto:Grub-devel@gnu.org> https://lists.gnu.org/mailman/listinfo/grub-devel -- Regards Vladimir 'phcoder' Serbinenko
_______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
