zlogger: improve initialization 63/280463/5
authorMateusz Majewski <m.majewski2@samsung.com>
Tue, 30 Aug 2022 09:55:00 +0000 (11:55 +0200)
committerMateusz Majewski <m.majewski2@samsung.com>
Wed, 31 Aug 2022 08:12:57 +0000 (10:12 +0200)
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 <m.majewski2@samsung.com>
kernel/zlogger/zlogger.c

index 64c7fcf..cfa3aad 100644 (file)
@@ -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);