* [PATCH] libata: fix probe_ent free in ata_sas_port_alloc() @ 2007-02-17 14:27 Tejun Heo 2007-02-17 15:16 ` James Bottomley 0 siblings, 1 reply; 9+ messages in thread From: Tejun Heo @ 2007-02-17 14:27 UTC (permalink / raw) To: Jeff Garzik, James Bottomley; +Cc: linux-ide, SCSI Mailing List probe_ent is allocated using devm_kzalloc() and thus should be freed using devm_kfree(). ata_sas_port_alloc() freed its probe_ent using kfree() thus causing double free later. Signed-off-by: Tejun Heo <htejun@gmail.com> --- James, does this fix the bug you mentioned on IRC? diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index 0009818..e5e19e3 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -3234,7 +3234,7 @@ struct ata_port *ata_sas_port_alloc(struct ata_host *host, ata_port_init(ap, host, ent, 0); ap->lock = shost->host_lock; - kfree(ent); + devm_kfree(host->dev, ent); return ap; } EXPORT_SYMBOL_GPL(ata_sas_port_alloc); ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] libata: fix probe_ent free in ata_sas_port_alloc() 2007-02-17 14:27 [PATCH] libata: fix probe_ent free in ata_sas_port_alloc() Tejun Heo @ 2007-02-17 15:16 ` James Bottomley 2007-02-17 17:24 ` [PATCH] libata: fix probe_ent alloc/free bugs Tejun Heo 0 siblings, 1 reply; 9+ messages in thread From: James Bottomley @ 2007-02-17 15:16 UTC (permalink / raw) To: Tejun Heo; +Cc: Jeff Garzik, linux-ide, SCSI Mailing List On Sat, 2007-02-17 at 23:27 +0900, Tejun Heo wrote: > probe_ent is allocated using devm_kzalloc() and thus should be freed > using devm_kfree(). ata_sas_port_alloc() freed its probe_ent using > kfree() thus causing double free later. > > Signed-off-by: Tejun Heo <htejun@gmail.com> > --- > James, does this fix the bug you mentioned on IRC? Yes and no. I actually have two devices in this sas setup: a SATA disk and a SATAPI DVD burner. Originally, I got the bug I reported here Subject: BUG in libata from ata_sas_port_alloc On my SATA disk. However, the DVD was fine. Now the disk shows up fine, but I get this from the DVD: BUG: at drivers/base/devres.c:642 devm_kfree() [<c0103c0a>] show_trace_log_lvl+0x1a/0x30 [<c0104282>] show_trace+0x12/0x20 [<c0104336>] dump_stack+0x16/0x20 [<c023f09a>] devm_kfree+0x4a/0x50 [<f892eea2>] ata_sas_port_alloc+0x62/0x80 [libata] [<f897869e>] sas_ata_init_host_and_port+0x5e/0xa0 [libsas] [<f897832d>] sas_target_alloc+0x4d/0x60 [libsas] [...] This time, it's the opposite problem: the SATAPI DVD was kmalloc allocated. The fault all seems to be in this code: struct ata_probe_ent * ata_probe_ent_alloc(struct device *dev, const struct ata_port_info *port) { struct ata_probe_ent *probe_ent; /* XXX - the following if can go away once all LLDs are managed */ if (!list_empty(&dev->devres_head)) probe_ent = devm_kzalloc(dev, sizeof(*probe_ent), GFP_KERNEL); else probe_ent = kzalloc(sizeof(*probe_ent), GFP_KERNEL); So we can't tell how the memory was obtained. To fix it, it looks like we might have to mark it in some way and then call a freeing function (ata_probe_ent_free?) to release it via the correct method. James ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] libata: fix probe_ent alloc/free bugs 2007-02-17 15:16 ` James Bottomley @ 2007-02-17 17:24 ` Tejun Heo 2007-02-17 17:43 ` Sergei Shtylyov ` (3 more replies) 0 siblings, 4 replies; 9+ messages in thread From: Tejun Heo @ 2007-02-17 17:24 UTC (permalink / raw) To: James Bottomley; +Cc: Jeff Garzik, linux-ide, SCSI Mailing List ata_probe_ent_alloc() had a temporary hack such that devm_kzalloc() was used for allocation if devres had been previously initialized on the device; otherwise, plain kzalloc() was used. This was to make the code useable from both the old and devres-aware libata drivers during transition. This hack made ata_sas_port_alloc() unable to determine how the probe_ent is allocated, causing double free in some cases. Remove the now-unneeded hack and make ata_sas_port_alloc() use devm_kfree(). Signed-off-by: Tejun Heo <htejun@gmail.com> Cc: James Bottomley <James.Bottomley@SteelEye.com> --- James, thanks for the diagnosis and please ack if this fixes the problem. Jeff, after James' ack, can you please verify this works with a libata driver? Just loading and unloading a libata LLD should be enough. I'm visiting my hometown for lunar new year's day, so I can't do it till Tuesday. Thanks. diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index 2cf8251..c34e0b4 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -5891,11 +5891,7 @@ ata_probe_ent_alloc(struct device *dev, const struct ata_port_info *port) { struct ata_probe_ent *probe_ent; - /* XXX - the following if can go away once all LLDs are managed */ - if (!list_empty(&dev->devres_head)) - probe_ent = devm_kzalloc(dev, sizeof(*probe_ent), GFP_KERNEL); - else - probe_ent = kzalloc(sizeof(*probe_ent), GFP_KERNEL); + probe_ent = devm_kzalloc(dev, sizeof(*probe_ent), GFP_KERNEL); if (!probe_ent) { printk(KERN_ERR DRV_NAME "(%s): out of memory\n", kobject_name(&(dev->kobj))); diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index 0009818..e5e19e3 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -3234,7 +3234,7 @@ struct ata_port *ata_sas_port_alloc(struct ata_host *host, ata_port_init(ap, host, ent, 0); ap->lock = shost->host_lock; - kfree(ent); + devm_kfree(host->dev, ent); return ap; } EXPORT_SYMBOL_GPL(ata_sas_port_alloc); ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] libata: fix probe_ent alloc/free bugs 2007-02-17 17:24 ` [PATCH] libata: fix probe_ent alloc/free bugs Tejun Heo @ 2007-02-17 17:43 ` Sergei Shtylyov 2007-02-17 18:19 ` James Bottomley 2007-02-17 18:09 ` James Bottomley ` (2 subsequent siblings) 3 siblings, 1 reply; 9+ messages in thread From: Sergei Shtylyov @ 2007-02-17 17:43 UTC (permalink / raw) To: Tejun Heo; +Cc: James Bottomley, Jeff Garzik, linux-ide, SCSI Mailing List Hello. Tejun Heo wrote: > ata_probe_ent_alloc() had a temporary hack such that devm_kzalloc() > was used for allocation if devres had been previously initialized on > the device; otherwise, plain kzalloc() was used. This was to make the > code useable from both the old and devres-aware libata drivers during > transition. This hack made ata_sas_port_alloc() unable to determine > how the probe_ent is allocated, causing double free in some cases. > > Remove the now-unneeded hack and make ata_sas_port_alloc() use > devm_kfree(). [...] > diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c > index 2cf8251..c34e0b4 100644 > --- a/drivers/ata/libata-core.c > +++ b/drivers/ata/libata-core.c > @@ -5891,11 +5891,7 @@ ata_probe_ent_alloc(struct device *dev, const > struct ata_port_info *port) > { > struct ata_probe_ent *probe_ent; > > - /* XXX - the following if can go away once all LLDs are managed */ > - if (!list_empty(&dev->devres_head)) > - probe_ent = devm_kzalloc(dev, sizeof(*probe_ent), GFP_KERNEL); > - else > - probe_ent = kzalloc(sizeof(*probe_ent), GFP_KERNEL); > + probe_ent = devm_kzalloc(dev, sizeof(*probe_ent), GFP_KERNEL); > if (!probe_ent) { > printk(KERN_ERR DRV_NAME "(%s): out of memory\n", > kobject_name(&(dev->kobj))); [...] The patch certainly looks mangled tab-wise. :-) MBR, Sergei ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] libata: fix probe_ent alloc/free bugs 2007-02-17 17:43 ` Sergei Shtylyov @ 2007-02-17 18:19 ` James Bottomley 2007-02-17 18:27 ` Sergei Shtylyov 0 siblings, 1 reply; 9+ messages in thread From: James Bottomley @ 2007-02-17 18:19 UTC (permalink / raw) To: Sergei Shtylyov; +Cc: Tejun Heo, Jeff Garzik, linux-ide, SCSI Mailing List On Sat, 2007-02-17 at 20:43 +0300, Sergei Shtylyov wrote: > > + probe_ent = devm_kzalloc(dev, sizeof(*probe_ent), GFP_KERNEL); > > if (!probe_ent) { > > printk(KERN_ERR DRV_NAME "(%s): out of memory\n", > > kobject_name(&(dev->kobj))); > [...] > > The patch certainly looks mangled tab-wise. :-) It isn't, though, so I think you'll find it's a problem with your mail client. James ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] libata: fix probe_ent alloc/free bugs 2007-02-17 18:19 ` James Bottomley @ 2007-02-17 18:27 ` Sergei Shtylyov 0 siblings, 0 replies; 9+ messages in thread From: Sergei Shtylyov @ 2007-02-17 18:27 UTC (permalink / raw) To: James Bottomley; +Cc: Tejun Heo, Jeff Garzik, linux-ide, SCSI Mailing List Hello. James Bottomley wrote: >>>+ probe_ent = devm_kzalloc(dev, sizeof(*probe_ent), GFP_KERNEL); >>> if (!probe_ent) { >>> printk(KERN_ERR DRV_NAME "(%s): out of memory\n", >>> kobject_name(&(dev->kobj))); >>[...] >> The patch certainly looks mangled tab-wise. :-) > It isn't, though, so I think you'll find it's a problem with your mail > client. You're right. :-< It seems to "dislike" format=flowed in this message (that's what is different from other mails where tabs are shown as is)... > James MBR, Sergei ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] libata: fix probe_ent alloc/free bugs 2007-02-17 17:24 ` [PATCH] libata: fix probe_ent alloc/free bugs Tejun Heo 2007-02-17 17:43 ` Sergei Shtylyov @ 2007-02-17 18:09 ` James Bottomley 2007-02-20 8:37 ` Tejun Heo 2007-02-20 15:52 ` Jeff Garzik 3 siblings, 0 replies; 9+ messages in thread From: James Bottomley @ 2007-02-17 18:09 UTC (permalink / raw) To: Tejun Heo; +Cc: Jeff Garzik, linux-ide, SCSI Mailing List On Sun, 2007-02-18 at 02:24 +0900, Tejun Heo wrote: > ata_probe_ent_alloc() had a temporary hack such that devm_kzalloc() > was used for allocation if devres had been previously initialized on > the device; otherwise, plain kzalloc() was used. This was to make the > code useable from both the old and devres-aware libata drivers during > transition. This hack made ata_sas_port_alloc() unable to determine > how the probe_ent is allocated, causing double free in some cases. > > Remove the now-unneeded hack and make ata_sas_port_alloc() use > devm_kfree(). > > Signed-off-by: Tejun Heo <htejun@gmail.com> > Cc: James Bottomley <James.Bottomley@SteelEye.com> > --- > James, thanks for the diagnosis and please ack if this fixes the > problem. Yes, it fixes all the problems. James ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] libata: fix probe_ent alloc/free bugs 2007-02-17 17:24 ` [PATCH] libata: fix probe_ent alloc/free bugs Tejun Heo 2007-02-17 17:43 ` Sergei Shtylyov 2007-02-17 18:09 ` James Bottomley @ 2007-02-20 8:37 ` Tejun Heo 2007-02-20 15:52 ` Jeff Garzik 3 siblings, 0 replies; 9+ messages in thread From: Tejun Heo @ 2007-02-20 8:37 UTC (permalink / raw) To: Jeff Garzik; +Cc: James Bottomley, linux-ide, SCSI Mailing List Tejun Heo wrote: > ata_probe_ent_alloc() had a temporary hack such that devm_kzalloc() > was used for allocation if devres had been previously initialized on > the device; otherwise, plain kzalloc() was used. This was to make the > code useable from both the old and devres-aware libata drivers during > transition. This hack made ata_sas_port_alloc() unable to determine > how the probe_ent is allocated, causing double free in some cases. > > Remove the now-unneeded hack and make ata_sas_port_alloc() use > devm_kfree(). > > Signed-off-by: Tejun Heo <htejun@gmail.com> > Cc: James Bottomley <James.Bottomley@SteelEye.com> > --- > James, thanks for the diagnosis and please ack if this fixes the > problem. > > Jeff, after James' ack, can you please verify this works with a libata > driver? Just loading and unloading a libata LLD should be enough. > I'm visiting my hometown for lunar new year's day, so I can't do it > till Tuesday. Okay, verified. Please apply. -- tejun ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] libata: fix probe_ent alloc/free bugs 2007-02-17 17:24 ` [PATCH] libata: fix probe_ent alloc/free bugs Tejun Heo ` (2 preceding siblings ...) 2007-02-20 8:37 ` Tejun Heo @ 2007-02-20 15:52 ` Jeff Garzik 3 siblings, 0 replies; 9+ messages in thread From: Jeff Garzik @ 2007-02-20 15:52 UTC (permalink / raw) To: Tejun Heo; +Cc: James Bottomley, linux-ide, SCSI Mailing List Tejun Heo wrote: > ata_probe_ent_alloc() had a temporary hack such that devm_kzalloc() > was used for allocation if devres had been previously initialized on > the device; otherwise, plain kzalloc() was used. This was to make the > code useable from both the old and devres-aware libata drivers during > transition. This hack made ata_sas_port_alloc() unable to determine > how the probe_ent is allocated, causing double free in some cases. > > Remove the now-unneeded hack and make ata_sas_port_alloc() use > devm_kfree(). > > Signed-off-by: Tejun Heo <htejun@gmail.com> > Cc: James Bottomley <James.Bottomley@SteelEye.com> > --- > James, thanks for the diagnosis and please ack if this fixes the > problem. > > Jeff, after James' ack, can you please verify this works with a libata > driver? Just loading and unloading a libata LLD should be enough. > I'm visiting my hometown for lunar new year's day, so I can't do it > till Tuesday. > > Thanks. applied ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2007-02-20 15:52 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2007-02-17 14:27 [PATCH] libata: fix probe_ent free in ata_sas_port_alloc() Tejun Heo 2007-02-17 15:16 ` James Bottomley 2007-02-17 17:24 ` [PATCH] libata: fix probe_ent alloc/free bugs Tejun Heo 2007-02-17 17:43 ` Sergei Shtylyov 2007-02-17 18:19 ` James Bottomley 2007-02-17 18:27 ` Sergei Shtylyov 2007-02-17 18:09 ` James Bottomley 2007-02-20 8:37 ` Tejun Heo 2007-02-20 15:52 ` Jeff Garzik
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.