From 628f34282db49359576dcb8cbaea65b4bf083ebd Mon Sep 17 00:00:00 2001 From: Christopher Brannon Date: Sun, 19 Dec 2010 22:50:24 +0000 Subject: [PATCH] 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 Acked-by: Dan Carpenter Signed-off-by: Greg Kroah-Hartman --- drivers/staging/speakup/main.c | 129 +++++++++++++++++++++++---------- 1 file changed, 89 insertions(+), 40 deletions(-) diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c index 3cd00396a462..cd981a13c12d 100644 --- 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; - for (i = 0; i < MAX_NR_CONSOLES; i++) - if (vc_cons[i].d) - speakup_allocate(vc_cons[i].d); + /* 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) { + 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; }