| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <1161627784.5182.151.camel@localhost.localdomain>
Date: Mon, 23 Oct 2006 11:23:04 -0700
From: Kylene Jo Hall <kjhall@...ibm.com>
To: Neil Horman <nhorman@...driver.com>
Cc: Alan Cox <alan@...rguk.ukuu.org.uk>,
kernel-janitors@...ts.osdl.org, akpm@...l.org,
benh@...nel.crashing.org, maxk@...lcomm.com,
linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [KJ][PATCH] Correct misc_register return code handling in
several drivers
For tpm:
Ack'ed-by: Kylene Hall <kjhall@...ibm.com>
On Mon, 2006-10-23 at 13:54 -0400, Neil Horman wrote:
> On Mon, Oct 23, 2006 at 06:39:24PM +0100, Alan Cox wrote:
> > Ar Llu, 2006-10-23 am 13:19 -0400, ysgrifennodd Neil Horman:
> > > + for_each_online_node(node) {
> > > + if(timers[node] != NULL)
> > > + kfree(timers[node]);
> > > + }
> >
> > The if test appears to be redundant as kfree(NULL) is a valid no-op.
> >
> > > + if (misc_register(&hp_sdc_rtc_dev) != 0)
> > > + printk(KERN_INFO "Could not register misc. dev for sdc\n");
> >
> > Many users will misconstrue this has a hard disk name - is there a
> > better name to use.
> >
> > > if (ret)
> > > printk(KERN_ERR "tun: Can't register misc device %d\n", TUN_MINOR);
> >
> > These all really show that we should pass the name into the misc
> > register and do the printk there to cut down on random name variants and
> > kernel size. Separate problem, just noting it in case someone feels
> > inspired 8)
> >
> > Alan
>
> New patch attached, taking Alan's suggestions into account. I'll consolidate
> the printk error messages into misc_register in a later patch when I have a
> moment. Thanks Alan.
>
> Regards
> Neil
>
>
> Signed-off-by: Neil Horman <nhorman@...driver.com>
>
> drivers/char/mmtimer.c | 28 ++++++++++++++++++++++------
> drivers/char/tpm/tpm.c | 1 +
> drivers/input/misc/hp_sdc_rtc.c | 6 +++++-
> drivers/macintosh/apm_emu.c | 5 ++++-
> drivers/net/tun.c | 2 ++
> fs/dlm/user.c | 1 +
> 6 files changed, 35 insertions(+), 8 deletions(-)
>
>
>
> diff --git a/drivers/char/mmtimer.c b/drivers/char/mmtimer.c
> --- a/drivers/char/mmtimer.c
> +++ b/drivers/char/mmtimer.c
> @@ -669,6 +669,7 @@ static struct k_clock sgi_clock = {
> static int __init mmtimer_init(void)
> {
> unsigned i;
> + int err = -1;
> cnodeid_t node, maxn = -1;
>
> if (!ia64_platform_is("sn2"))
> @@ -680,7 +681,7 @@ static int __init mmtimer_init(void)
> if (sn_rtc_cycles_per_second < 100000) {
> printk(KERN_ERR "%s: unable to determine clock frequency\n",
> MMTIMER_NAME);
> - return -1;
> + goto out;
> }
>
> mmtimer_femtoperiod = ((unsigned long)1E15 + sn_rtc_cycles_per_second /
> @@ -689,13 +690,13 @@ static int __init mmtimer_init(void)
> if (request_irq(SGI_MMTIMER_VECTOR, mmtimer_interrupt, IRQF_PERCPU, MMTIMER_NAME, NULL)) {
> printk(KERN_WARNING "%s: unable to allocate interrupt.",
> MMTIMER_NAME);
> - return -1;
> + goto out;
> }
>
> if (misc_register(&mmtimer_miscdev)) {
> printk(KERN_ERR "%s: failed to register device\n",
> MMTIMER_NAME);
> - return -1;
> + goto out1;
> }
>
> /* Get max numbered node, calculate slots needed */
> @@ -709,16 +710,18 @@ static int __init mmtimer_init(void)
> if (timers == NULL) {
> printk(KERN_ERR "%s: failed to allocate memory for device\n",
> MMTIMER_NAME);
> - return -1;
> + goto out2;
> }
>
> + memset(timers,0,(sizeof(mmtimer_t *)*maxn));
> +
> /* Allocate mmtimer_t's for each online node */
> for_each_online_node(node) {
> timers[node] = kmalloc_node(sizeof(mmtimer_t)*NUM_COMPARATORS, GFP_KERNEL, node);
> if (timers[node] == NULL) {
> printk(KERN_ERR "%s: failed to allocate memory for device\n",
> MMTIMER_NAME);
> - return -1;
> + goto out3;
> }
> for (i=0; i< NUM_COMPARATORS; i++) {
> mmtimer_t * base = timers[node] + i;
> @@ -738,7 +741,20 @@ static int __init mmtimer_init(void)
> printk(KERN_INFO "%s: v%s, %ld MHz\n", MMTIMER_DESC, MMTIMER_VERSION,
> sn_rtc_cycles_per_second/(unsigned long)1E6);
>
> - return 0;
> + err = 0;
> +out:
> + return err;
> +
> +out3:
> + for_each_online_node(node) {
> + kfree(timers[node]);
> + }
> +out2:
> + misc_deregister(&mmtimer_miscdev);
> +out1:
> + free_irq(SGI_MMTIMER_VECTOR, NULL);
> + goto out;
> +
> }
>
> module_init(mmtimer_init);
> diff --git a/drivers/char/tpm/tpm.c b/drivers/char/tpm/tpm.c
> --- a/drivers/char/tpm/tpm.c
> +++ b/drivers/char/tpm/tpm.c
> @@ -1155,6 +1155,7 @@ #define DEVNAME_SIZE 7
>
> if (sysfs_create_group(&dev->kobj, chip->vendor.attr_group)) {
> list_del(&chip->list);
> + misc_deregister(&chip->vendor.miscdev);
> put_device(dev);
> clear_bit(chip->dev_num, dev_mask);
> kfree(chip);
> diff --git a/drivers/input/misc/hp_sdc_rtc.c b/drivers/input/misc/hp_sdc_rtc.c
> --- a/drivers/input/misc/hp_sdc_rtc.c
> +++ b/drivers/input/misc/hp_sdc_rtc.c
> @@ -693,9 +693,13 @@ static int __init hp_sdc_rtc_init(void)
>
> init_MUTEX(&i8042tregs);
>
> + INIT_LIST_HEAD(&hp_sdc_rtc_dev.list);
> +
> if ((ret = hp_sdc_request_timer_irq(&hp_sdc_rtc_isr)))
> return ret;
> - misc_register(&hp_sdc_rtc_dev);
> + if (misc_register(&hp_sdc_rtc_dev) != 0)
> + printk(KERN_INFO "Could not register misc. dev for i8042 rtc\n");
> +
> create_proc_read_entry ("driver/rtc", 0, NULL,
> hp_sdc_rtc_read_proc, NULL);
>
> diff --git a/drivers/macintosh/apm_emu.c b/drivers/macintosh/apm_emu.c
> --- a/drivers/macintosh/apm_emu.c
> +++ b/drivers/macintosh/apm_emu.c
> @@ -520,6 +520,8 @@ static int __init apm_emu_init(void)
> {
> struct proc_dir_entry *apm_proc;
>
> + INIT_LIST_HEAD(&apm_device.list);
> +
> if (sys_ctrler != SYS_CTRLER_PMU) {
> printk(KERN_INFO "apm_emu: Requires a machine with a PMU.\n");
> return -ENODEV;
> @@ -529,7 +531,8 @@ static int __init apm_emu_init(void)
> if (apm_proc)
> apm_proc->owner = THIS_MODULE;
>
> - misc_register(&apm_device);
> + if (misc_register(&apm_device) != 0)
> + printk(KERN_INFO "Could not create misc. device for apm\n");
>
> pmu_register_sleep_notifier(&apm_sleep_notifier);
>
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -856,6 +856,8 @@ static int __init tun_init(void)
> printk(KERN_INFO "tun: %s, %s\n", DRV_DESCRIPTION, DRV_VERSION);
> printk(KERN_INFO "tun: %s\n", DRV_COPYRIGHT);
>
> + INIT_LIST_HEAD(&tun_miscdev.list);
> +
> ret = misc_register(&tun_miscdev);
> if (ret)
> printk(KERN_ERR "tun: Can't register misc device %d\n", TUN_MINOR);
> diff --git a/fs/dlm/user.c b/fs/dlm/user.c
> --- a/fs/dlm/user.c
> +++ b/fs/dlm/user.c
> @@ -773,6 +773,7 @@ int dlm_user_init(void)
> ctl_device.name = "dlm-control";
> ctl_device.fops = &ctl_device_fops;
> ctl_device.minor = MISC_DYNAMIC_MINOR;
> + INIT_LIST_HEAD(&ctl_device.list);
>
> error = misc_register(&ctl_device);
> if (error)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists