From 37bf37e9868ea641cf7e282e0eab12dda70642e4 Mon Sep 17 00:00:00 2001 From: Mateusz Majewski Date: Tue, 30 Aug 2022 11:55:00 +0200 Subject: [PATCH] zlogger: improve initialization The original code used an incorrect order of initialization. It's important to first initialize the module variables, and only afterwards initialize the device. Otherwise, userspace might access the device before it is ready. Additionally, some missing failure handling is added. Change-Id: I8c907b92c5c45568e44fdbb0e645e0b593fb898c Signed-off-by: Mateusz Majewski --- kernel/zlogger/zlogger.c | 52 +++++++++++++++++++++++----------------- 1 file changed, 30 insertions(+), 22 deletions(-) diff --git a/kernel/zlogger/zlogger.c b/kernel/zlogger/zlogger.c index 64c7fcf..cfa3aad 100644 --- a/kernel/zlogger/zlogger.c +++ b/kernel/zlogger/zlogger.c @@ -744,25 +744,6 @@ int __init zlogger_init(void) return 0; } - zlogger_device.minor = MISC_DYNAMIC_MINOR; - zlogger_device.name = ZLOGGER_DEVICE_NAME; - zlogger_device.fops = &zlogger_fops; - zlogger_device.mode = 0666; -#ifdef CONFIG_SECURITY_SMACK_SET_DEV_SMK_LABEL - zlogger_device.lab_smk64 = ZLOGGER_SMACK_LABEL; -#endif - r = misc_register(&zlogger_device); - if (unlikely(r)) { - pr_err("Failed to register misc device for '%s' (%d)\n", ZLOGGER_DEVICE_NAME, r); - return r; - } - - r = sysfs_create_group(&zlogger_device.this_device->kobj, &zlogger_attr_group); - if (unlikely(r)) { - dev_err(zlogger_device.this_device, "failed to create sysfs nodes with (%d)\n", r); - return r; - } - g_thread_table = kzalloc(sizeof(struct thread_table), GFP_KERNEL); if (g_thread_table == NULL) return -ENOMEM; @@ -790,11 +771,38 @@ int __init zlogger_init(void) #endif run_task(); + zlogger_device.minor = MISC_DYNAMIC_MINOR; + zlogger_device.name = ZLOGGER_DEVICE_NAME; + zlogger_device.fops = &zlogger_fops; + zlogger_device.mode = 0666; +#ifdef CONFIG_SECURITY_SMACK_SET_DEV_SMK_LABEL + zlogger_device.lab_smk64 = ZLOGGER_SMACK_LABEL; +#endif + r = misc_register(&zlogger_device); + if (unlikely(r)) { + pr_err("Failed to register misc device for '%s' (%d)\n", ZLOGGER_DEVICE_NAME, r); + goto out_free_zlog_task; + } + + r = sysfs_create_group(&zlogger_device.this_device->kobj, &zlogger_attr_group); + if (unlikely(r)) { + dev_err(zlogger_device.this_device, "failed to create sysfs nodes with (%d)\n", r); + goto out_free_zlogger_device; + } + g_init = 1; pr_info("Init success\n"); return 0; +out_free_zlogger_device: + misc_deregister(&zlogger_device); + +out_free_zlog_task: + /* TODO: actually destroy this task. + * We don't do so yet, even in zlogger_exit (see TODO there). + */ + out_free_g_thread_table_g_shm_ptr: for (i = 0; i < g_shm_ptr_i; ++i) { kfree(g_shm_ptr[i]); @@ -814,6 +822,9 @@ static void __exit zlogger_exit(void) struct hlist_node *tmp_iter = NULL; int tmp_bkt; + sysfs_remove_group(&zlogger_device.this_device->kobj, &zlogger_attr_group); + misc_deregister(&zlogger_device); + // TODO: What about the task that is running in the background? queue_deinit(&g_free_q); @@ -830,9 +841,6 @@ static void __exit zlogger_exit(void) } g_init = 0; - - sysfs_remove_group(&zlogger_device.this_device->kobj, &zlogger_attr_group); - misc_deregister(&zlogger_device); } module_init(zlogger_init); -- 2.34.1