staging: speakup: more fixes for init-failure handling.

We still leaked many resources when Speakup failed to initialize.
Examples of leaked resources include:
/dev/synth, keyboard or VT notifiers, and heap-allocated st_spk_t
structs.
This is fixed.

* We now use PTR_ERR to detect kthread_create failure
(thank you Dan Carpenter).

* The loop which frees members of the speakup_console array now iterates
over the whole array, not stopping at the first NULL value.  Fixes
a possible memory leak.  Safe because kfree(NULL) is a no-op.

* The order of some initializations was changed.  The safe ones, which
will never fail, are performed first.

Signed-off-by: Christopher Brannon <chris@the-brannons.com>
Acked-by: Dan Carpenter <error27@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c
index 3cd00396a462baf3d3837ca192778c25f746c683..cd981a13c12d49074a480c3094b07bbbbf0dcbe2 100644 (file)
--- a/drivers/staging/speakup/main.c
+++ b/drivers/staging/speakup/main.c
@@ -1283,7 +1283,7 @@ static int edit_bits(struct vc_data *vc, u_char type, u_char ch, u_short key)
 }
 
 /* Allocation concurrency is protected by the console semaphore */
-void speakup_allocate(struct vc_data *vc)
+int speakup_allocate(struct vc_data *vc)
 {
        int vc_num;
 
@@ -1292,10 +1292,12 @@ void speakup_allocate(struct vc_data *vc)
                speakup_console[vc_num] = kzalloc(sizeof(*speakup_console[0]),
                                                  GFP_ATOMIC);
                if (speakup_console[vc_num] == NULL)
-                       return;
+                       return -ENOMEM;
                speakup_date(vc);
        } else if (!spk_parked)
                speakup_date(vc);
+
+       return 0;
 }
 
 void speakup_deallocate(struct vc_data *vc)
@@ -2217,18 +2219,23 @@ static void __exit speakup_exit(void)
 {
        int i;
 
-       free_user_msgs();
        unregister_keyboard_notifier(&keyboard_notifier_block);
        unregister_vt_notifier(&vt_notifier_block);
        speakup_unregister_devsynth();
        del_timer(&cursor_timer);
-
        kthread_stop(speakup_task);
        speakup_task = NULL;
        mutex_lock(&spk_mutex);
        synth_release();
        mutex_unlock(&spk_mutex);
 
+       speakup_kobj_exit();
+
+       for (i = 0; i < MAX_NR_CONSOLES; i++)
+               kfree(speakup_console[i]);
+
+       speakup_remove_virtual_keyboard();
+
        for (i = 0; i < MAXVARS; i++)
                speakup_unregister_var(i);
 
@@ -2236,42 +2243,23 @@ static void __exit speakup_exit(void)
                if (characters[i] != default_chars[i])
                        kfree(characters[i]);
        }
-       for (i = 0; speakup_console[i]; i++)
-               kfree(speakup_console[i]);
-       speakup_kobj_exit();
-       speakup_remove_virtual_keyboard();
+
+       free_user_msgs();
 }
 
 /* call by: module_init() */
 static int __init speakup_init(void)
 {
        int i;
-       int err;
+       long err = 0;
        struct st_spk_t *first_console;
        struct vc_data *vc = vc_cons[fg_console].d;
        struct var_t *var;
 
-       err = speakup_add_virtual_keyboard();
-       if (err)
-               goto out;
-
+       /* These first few initializations cannot fail. */
        initialize_msgs();      /* Initialize arrays for i18n. */
-       first_console = kzalloc(sizeof(*first_console), GFP_KERNEL);
-       if (!first_console) {
-               err = -ENOMEM;
-               goto err_cons;
-       }
-       err = speakup_kobj_init();
-       if (err)
-               goto err_kobject;
-
        reset_default_chars();
        reset_default_chartab();
-
-       speakup_console[vc->vc_num] = first_console;
-       speakup_date(vc);
-       pr_info("speakup %s: initialized\n", SPEAKUP_VERSION);
-
        strlwr(synth_name);
        spk_vars[0].u.n.high = vc->vc_cols;
        for (var = spk_vars; var->var_id != MAXVARS; var++)
@@ -2286,31 +2274,92 @@ static int __init speakup_init(void)
        if (quiet_boot)
                spk_shut_up |= 0x01;
 
+       /* From here on out, initializations can fail. */
+       err = speakup_add_virtual_keyboard();
+       if (err)
+               goto error_virtkeyboard;
+
+       first_console = kzalloc(sizeof(*first_console), GFP_KERNEL);
+       if (!first_console) {
+               err = -ENOMEM;
+               goto error_alloc;
+       }
+
+       speakup_console[vc->vc_num] = first_console;
+       speakup_date(vc);
+
        for (i = 0; i < MAX_NR_CONSOLES; i++)
-               if (vc_cons[i].d)
-                       speakup_allocate(vc_cons[i].d);
+               if (vc_cons[i].d) {
+                       err = speakup_allocate(vc_cons[i].d);
+                       if (err)
+                               goto error_kobjects;
+               }
+
+       err = speakup_kobj_init();
+       if (err)
+               goto error_kobjects;
 
-       pr_warn("synth name on entry is: %s\n", synth_name);
        synth_init(synth_name);
        speakup_register_devsynth();
+       /*
+        * register_devsynth might fail, but this error is not fatal.
+        * /dev/synth is an extra feature; the rest of Speakup
+        * will work fine without it.
+        */
 
-       register_keyboard_notifier(&keyboard_notifier_block);
-       register_vt_notifier(&vt_notifier_block);
+       err = register_keyboard_notifier(&keyboard_notifier_block);
+       if (err)
+               goto error_kbdnotifier;
+       err = register_vt_notifier(&vt_notifier_block);
+       if (err)
+               goto error_vtnotifier;
 
        speakup_task = kthread_create(speakup_thread, NULL, "speakup");
-       set_user_nice(speakup_task, 10);
+
        if (IS_ERR(speakup_task)) {
-               err = -ENOMEM;
-               goto err_kobject;
+               err = PTR_ERR(speakup_task);
+               goto error_task;
        }
+
+       set_user_nice(speakup_task, 10);
        wake_up_process(speakup_task);
+
+       pr_info("speakup %s: initialized\n", SPEAKUP_VERSION);
+       pr_info("synth name on entry is: %s\n", synth_name);
        goto out;
 
-err_kobject:
-speakup_kobj_exit();
-       kfree(first_console);
-err_cons:
+error_task:
+       unregister_vt_notifier(&vt_notifier_block);
+
+error_vtnotifier:
+       unregister_keyboard_notifier(&keyboard_notifier_block);
+       del_timer(&cursor_timer);
+
+error_kbdnotifier:
+       speakup_unregister_devsynth();
+       mutex_lock(&spk_mutex);
+       synth_release();
+       mutex_unlock(&spk_mutex);
+       speakup_kobj_exit();
+
+error_kobjects:
+       for (i = 0; i < MAX_NR_CONSOLES; i++)
+               kfree(speakup_console[i]);
+
+error_alloc:
        speakup_remove_virtual_keyboard();
+
+error_virtkeyboard:
+       for (i = 0; i < MAXVARS; i++)
+               speakup_unregister_var(i);
+
+       for (i = 0; i < 256; i++) {
+               if (characters[i] != default_chars[i])
+                       kfree(characters[i]);
+       }
+
+       free_user_msgs();
+
 out:
        return err;
 }