Messages in this thread Patches in this message | | | Subject | Re: [ATM] [UPDATE] unbalanced exit path in Forerunner HE he_init_one() (and an iphase patch too!) | Date | Fri, 09 May 2003 19:38:34 -0400 | From | chas williams <> |
| |
>- pci_enable_device() balanced on error path in he_init_one(); >- return of atm_dev_register() isn't lost any more if he_dev allocation fails > in he_init_one(); >- pci_disable_device() added to he_disable_one();
i hope you dont mind, but i really dislike the goto. one is plenty. attached is a cleanup for the iphase (i made a small addition -- i believe the MEMDUMP_XXXXXXREG cases are also guilty of dereferencing).
[ATM]: unbalanced exit path in Forerunner HE he_init_one(). thanks to Francois Romieu <romieu@fr.zoreil.com>
--- linux-2.5.68/drivers/atm/he.c.002 Fri May 9 15:33:08 2003 +++ linux-2.5.68/drivers/atm/he.c Fri May 9 19:00:02 2003 @@ -362,43 +362,54 @@ static int __devinit he_init_one(struct pci_dev *pci_dev, const struct pci_device_id *pci_ent) { - struct atm_dev *atm_dev; - struct he_dev *he_dev; + struct atm_dev *atm_dev = NULL; + struct he_dev *he_dev = NULL; + int err = 0; printk(KERN_INFO "he: %s\n", version); if (pci_enable_device(pci_dev)) return -EIO; - if (pci_set_dma_mask(pci_dev, HE_DMA_MASK) != 0) - { + if (pci_set_dma_mask(pci_dev, HE_DMA_MASK) != 0) { printk(KERN_WARNING "he: no suitable dma available\n"); - return -EIO; + err = -EIO; + goto init_one_failure; } atm_dev = atm_dev_register(DEV_LABEL, &he_ops, -1, 0); - if (!atm_dev) return -ENODEV; + if (!atm_dev) { + err = -ENODEV; + goto init_one_failure; + } pci_set_drvdata(pci_dev, atm_dev); he_dev = (struct he_dev *) kmalloc(sizeof(struct he_dev), GFP_KERNEL); - if (!he_dev) return -ENOMEM; + if (!he_dev) { + err = -ENOMEM; + goto init_one_failure; + } memset(he_dev, 0, sizeof(struct he_dev)); he_dev->pci_dev = pci_dev; he_dev->atm_dev = atm_dev; he_dev->atm_dev->dev_data = he_dev; HE_DEV(atm_dev) = he_dev; - he_dev->number = atm_dev->number; /* was devs */ + he_dev->number = atm_dev->number; if (he_start(atm_dev)) { - atm_dev_deregister(atm_dev); he_stop(he_dev); - kfree(he_dev); - return -ENODEV; + err = -ENODEV; + goto init_one_failure; } he_dev->next = NULL; if (he_devs) he_dev->next = he_devs; he_devs = he_dev; - return 0; + +init_one_failure: + if (atm_dev) atm_dev_deregister(atm_dev); + if (he_dev) kfree(he_dev); + pci_disable_device(pci_dev); + return err; } static void __devexit @@ -417,6 +428,7 @@ kfree(he_dev); pci_set_drvdata(pci_dev, NULL); + pci_disable_device(pci_dev); } [ATM]: Sneak variant of "ioremap() return dereferencing". thanks to Francois Romieu <romieu@fr.zoreil.com>
--- linux-2.5.68/drivers/atm/iphase.c.002 Fri May 9 19:01:27 2003 +++ linux-2.5.68/drivers/atm/iphase.c Fri May 9 19:35:24 2003 @@ -2774,7 +2774,7 @@ if (!capable(CAP_NET_ADMIN)) return -EPERM; tmps = (u16 *)ia_cmds.buf; for(i=0; i<0x80; i+=2, tmps++) - if(put_user(*(u16*)(iadev->seg_reg+i), tmps)) return -EFAULT; + if(put_user((u16)(readl(iadev->seg_reg+i) & 0xffff), tmps)) return -EFAULT; ia_cmds.status = 0; ia_cmds.len = 0x80; break; @@ -2782,7 +2782,7 @@ if (!capable(CAP_NET_ADMIN)) return -EPERM; tmps = (u16 *)ia_cmds.buf; for(i=0; i<0x80; i+=2, tmps++) - if(put_user(*(u16*)(iadev->reass_reg+i), tmps)) return -EFAULT; + if(put_user((u16)(readl(iadev->reass_reg+i) & 0xffff), tmps)) return -EFAULT; ia_cmds.status = 0; ia_cmds.len = 0x80; break; @@ -2799,10 +2799,10 @@ rfL = ®s_local->rfredn; /* Copy real rfred registers into the local copy */ for (i=0; i<(sizeof (rfredn_t))/4; i++) - ((u_int *)rfL)[i] = ((u_int *)iadev->reass_reg)[i] & 0xffff; + ((u_int *)rfL)[i] = readl(iadev->reass_reg + i) & 0xffff; /* Copy real ffred registers into the local copy */ for (i=0; i<(sizeof (ffredn_t))/4; i++) - ((u_int *)ffL)[i] = ((u_int *)iadev->seg_reg)[i] & 0xffff; + ((u_int *)ffL)[i] = readl(iadev->seg_reg + i) & 0xffff; if (copy_to_user(ia_cmds.buf, regs_local,sizeof(ia_regs_t))) { kfree(regs_local); - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |