* [PATCH 5/5] drivers/serial: Return -ENOMEM on memory allocation failure
@ 2010-08-11 10:11 ` Julia Lawall
0 siblings, 0 replies; 6+ messages in thread
From: Julia Lawall @ 2010-08-11 10:11 UTC (permalink / raw)
To: Pat Gefre, linux-ia64, linux-mips, linux-kernel, kernel-janitors
From: Julia Lawall <julia@diku.dk>
In this code, 0 is returned on memory allocation failure, even though other
failures return -ENOMEM or other similar values.
A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)
// <smpl>
@@
expression ret;
expression x,e1,e2,e3;
@@
ret = 0
... when != ret = e1
*x = \(kmalloc\|kcalloc\|kzalloc\)(...)
... when != ret = e2
if (x == NULL) { ... when != ret = e3
return ret;
}
// </smpl>
Signed-off-by: Julia Lawall <julia@diku.dk>
---
I believe this code also leaks earlier instances of port, which are only
referenced by card_ptr, which is freed in the error handling code at the
end of the function. A lot of operations are done on port on each
iteration, however, so I'm not sure whether it is good enough to just free
them. Perhaps there is some way to call ioc3uart_remove?
drivers/serial/ioc3_serial.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/serial/ioc3_serial.c b/drivers/serial/ioc3_serial.c
index 93de907..800c546 100644
--- a/drivers/serial/ioc3_serial.c
+++ b/drivers/serial/ioc3_serial.c
@@ -2044,6 +2044,7 @@ ioc3uart_probe(struct ioc3_submodule *is, struct ioc3_driver_data *idd)
if (!port) {
printk(KERN_WARNING
"IOC3 serial memory not available for port\n");
+ ret = -ENOMEM;
goto out4;
}
spin_lock_init(&port->ip_lock);
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 5/5] drivers/serial: Return -ENOMEM on memory allocation
@ 2010-08-11 10:11 ` Julia Lawall
0 siblings, 0 replies; 6+ messages in thread
From: Julia Lawall @ 2010-08-11 10:11 UTC (permalink / raw)
To: Pat Gefre, linux-ia64, linux-mips, linux-kernel, kernel-janitors
From: Julia Lawall <julia@diku.dk>
In this code, 0 is returned on memory allocation failure, even though other
failures return -ENOMEM or other similar values.
A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)
// <smpl>
@@
expression ret;
expression x,e1,e2,e3;
@@
ret = 0
... when != ret = e1
*x = \(kmalloc\|kcalloc\|kzalloc\)(...)
... when != ret = e2
if (x = NULL) { ... when != ret = e3
return ret;
}
// </smpl>
Signed-off-by: Julia Lawall <julia@diku.dk>
---
I believe this code also leaks earlier instances of port, which are only
referenced by card_ptr, which is freed in the error handling code at the
end of the function. A lot of operations are done on port on each
iteration, however, so I'm not sure whether it is good enough to just free
them. Perhaps there is some way to call ioc3uart_remove?
drivers/serial/ioc3_serial.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/serial/ioc3_serial.c b/drivers/serial/ioc3_serial.c
index 93de907..800c546 100644
--- a/drivers/serial/ioc3_serial.c
+++ b/drivers/serial/ioc3_serial.c
@@ -2044,6 +2044,7 @@ ioc3uart_probe(struct ioc3_submodule *is, struct ioc3_driver_data *idd)
if (!port) {
printk(KERN_WARNING
"IOC3 serial memory not available for port\n");
+ ret = -ENOMEM;
goto out4;
}
spin_lock_init(&port->ip_lock);
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 5/5] drivers/serial: Return -ENOMEM on memory allocation failure
2010-08-11 10:11 ` [PATCH 5/5] drivers/serial: Return -ENOMEM on memory allocation Julia Lawall
@ 2010-08-11 15:59 ` Patrick Gefre
-1 siblings, 0 replies; 6+ messages in thread
From: Patrick Gefre @ 2010-08-11 15:59 UTC (permalink / raw)
To: Julia Lawall; +Cc: linux-ia64, linux-mips, linux-kernel, kernel-janitors
Julia Lawall wrote:
> From: Julia Lawall <julia@diku.dk>
>
> In this code, 0 is returned on memory allocation failure, even though other
> failures return -ENOMEM or other similar values.
>
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @@
> expression ret;
> expression x,e1,e2,e3;
> @@
>
> ret = 0
> ... when != ret = e1
> *x = \(kmalloc\|kcalloc\|kzalloc\)(...)
> ... when != ret = e2
> if (x == NULL) { ... when != ret = e3
> return ret;
> }
> // </smpl>
>
> Signed-off-by: Julia Lawall <julia@diku.dk>
>
Signed-off-by: Pat Gefre <pfg@sgi.com>
> ---
> I believe this code also leaks earlier instances of port, which are only
> referenced by card_ptr, which is freed in the error handling code at the
> end of the function. A lot of operations are done on port on each
> iteration, however, so I'm not sure whether it is good enough to just free
> them. Perhaps there is some way to call ioc3uart_remove?
>
Yes you are right, there should be something like this for out4:
out4:
for (phys_port = 0; phys_port < PORTS_PER_CARD; phys_port++) {
port = card_ptr->ic_port[phys_port].icp_port;
if (port) {
pci_free_consistent(port->ip_idd->pdev,
TOTAL_RING_BUF_SIZE,
(void *)port->ip_cpu_ringbuf,
port->ip_dma_ringbuf);
kfree(port);
}
}
kfree(card_ptr);
return ret;
> drivers/serial/ioc3_serial.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/serial/ioc3_serial.c b/drivers/serial/ioc3_serial.c
> index 93de907..800c546 100644
> --- a/drivers/serial/ioc3_serial.c
> +++ b/drivers/serial/ioc3_serial.c
> @@ -2044,6 +2044,7 @@ ioc3uart_probe(struct ioc3_submodule *is, struct ioc3_driver_data *idd)
> if (!port) {
> printk(KERN_WARNING
> "IOC3 serial memory not available for port\n");
> + ret = -ENOMEM;
> goto out4;
> }
> spin_lock_init(&port->ip_lock);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 5/5] drivers/serial: Return -ENOMEM on memory allocation
@ 2010-08-11 15:59 ` Patrick Gefre
0 siblings, 0 replies; 6+ messages in thread
From: Patrick Gefre @ 2010-08-11 15:59 UTC (permalink / raw)
To: Julia Lawall; +Cc: linux-ia64, linux-mips, linux-kernel, kernel-janitors
Julia Lawall wrote:
> From: Julia Lawall <julia@diku.dk>
>
> In this code, 0 is returned on memory allocation failure, even though other
> failures return -ENOMEM or other similar values.
>
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @@
> expression ret;
> expression x,e1,e2,e3;
> @@
>
> ret = 0
> ... when != ret = e1
> *x = \(kmalloc\|kcalloc\|kzalloc\)(...)
> ... when != ret = e2
> if (x = NULL) { ... when != ret = e3
> return ret;
> }
> // </smpl>
>
> Signed-off-by: Julia Lawall <julia@diku.dk>
>
Signed-off-by: Pat Gefre <pfg@sgi.com>
> ---
> I believe this code also leaks earlier instances of port, which are only
> referenced by card_ptr, which is freed in the error handling code at the
> end of the function. A lot of operations are done on port on each
> iteration, however, so I'm not sure whether it is good enough to just free
> them. Perhaps there is some way to call ioc3uart_remove?
>
Yes you are right, there should be something like this for out4:
out4:
for (phys_port = 0; phys_port < PORTS_PER_CARD; phys_port++) {
port = card_ptr->ic_port[phys_port].icp_port;
if (port) {
pci_free_consistent(port->ip_idd->pdev,
TOTAL_RING_BUF_SIZE,
(void *)port->ip_cpu_ringbuf,
port->ip_dma_ringbuf);
kfree(port);
}
}
kfree(card_ptr);
return ret;
> drivers/serial/ioc3_serial.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/serial/ioc3_serial.c b/drivers/serial/ioc3_serial.c
> index 93de907..800c546 100644
> --- a/drivers/serial/ioc3_serial.c
> +++ b/drivers/serial/ioc3_serial.c
> @@ -2044,6 +2044,7 @@ ioc3uart_probe(struct ioc3_submodule *is, struct ioc3_driver_data *idd)
> if (!port) {
> printk(KERN_WARNING
> "IOC3 serial memory not available for port\n");
> + ret = -ENOMEM;
> goto out4;
> }
> spin_lock_init(&port->ip_lock);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 5/5] drivers/serial: Return -ENOMEM on memory allocation failure
2010-08-11 15:59 ` [PATCH 5/5] drivers/serial: Return -ENOMEM on memory allocation Patrick Gefre
@ 2010-08-11 16:23 ` Julia Lawall
-1 siblings, 0 replies; 6+ messages in thread
From: Julia Lawall @ 2010-08-11 16:23 UTC (permalink / raw)
To: Patrick Gefre; +Cc: linux-ia64, linux-mips, linux-kernel, kernel-janitors
> > I believe this code also leaks earlier instances of port, which are only
> > referenced by card_ptr, which is freed in the error handling code at the
> > end of the function. A lot of operations are done on port on each
> > iteration, however, so I'm not sure whether it is good enough to just free
> > them. Perhaps there is some way to call ioc3uart_remove?
> >
>
> Yes you are right, there should be something like this for out4:
>
> out4:
> for (phys_port = 0; phys_port < PORTS_PER_CARD; phys_port++) {
> port = card_ptr->ic_port[phys_port].icp_port;
> if (port) {
> pci_free_consistent(port->ip_idd->pdev,
> TOTAL_RING_BUF_SIZE,
> (void *)port->ip_cpu_ringbuf,
> port->ip_dma_ringbuf);
> kfree(port);
> }
> }
> kfree(card_ptr);
> return ret;
Actually, pci_alloc_consistent is only called when phys_port is 0. In the
subsequent cases, the ip_dma_ringbuf field is just initialized to the
previous value. So it could be:
out4:
for (phys_port = 0; phys_port < PORTS_PER_CARD; phys_port++) {
port = card_ptr->ic_port[phys_port].icp_port;
if (port) {
if (phys_port == 0)
pci_free_consistent(port->ip_idd->pdev,
TOTAL_RING_BUF_SIZE,
(void *)port->ip_cpu_ringbuf,
port->ip_dma_ringbuf);
kfree(port);
}
}
kfree(card_ptr);
return ret;
julia
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 5/5] drivers/serial: Return -ENOMEM on memory allocation
@ 2010-08-11 16:23 ` Julia Lawall
0 siblings, 0 replies; 6+ messages in thread
From: Julia Lawall @ 2010-08-11 16:23 UTC (permalink / raw)
To: Patrick Gefre; +Cc: linux-ia64, linux-mips, linux-kernel, kernel-janitors
> > I believe this code also leaks earlier instances of port, which are only
> > referenced by card_ptr, which is freed in the error handling code at the
> > end of the function. A lot of operations are done on port on each
> > iteration, however, so I'm not sure whether it is good enough to just free
> > them. Perhaps there is some way to call ioc3uart_remove?
> >
>
> Yes you are right, there should be something like this for out4:
>
> out4:
> for (phys_port = 0; phys_port < PORTS_PER_CARD; phys_port++) {
> port = card_ptr->ic_port[phys_port].icp_port;
> if (port) {
> pci_free_consistent(port->ip_idd->pdev,
> TOTAL_RING_BUF_SIZE,
> (void *)port->ip_cpu_ringbuf,
> port->ip_dma_ringbuf);
> kfree(port);
> }
> }
> kfree(card_ptr);
> return ret;
Actually, pci_alloc_consistent is only called when phys_port is 0. In the
subsequent cases, the ip_dma_ringbuf field is just initialized to the
previous value. So it could be:
out4:
for (phys_port = 0; phys_port < PORTS_PER_CARD; phys_port++) {
port = card_ptr->ic_port[phys_port].icp_port;
if (port) {
if (phys_port = 0)
pci_free_consistent(port->ip_idd->pdev,
TOTAL_RING_BUF_SIZE,
(void *)port->ip_cpu_ringbuf,
port->ip_dma_ringbuf);
kfree(port);
}
}
kfree(card_ptr);
return ret;
julia
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-08-11 16:23 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-08-11 10:11 [PATCH 5/5] drivers/serial: Return -ENOMEM on memory allocation failure Julia Lawall
2010-08-11 10:11 ` [PATCH 5/5] drivers/serial: Return -ENOMEM on memory allocation Julia Lawall
2010-08-11 15:59 ` [PATCH 5/5] drivers/serial: Return -ENOMEM on memory allocation failure Patrick Gefre
2010-08-11 15:59 ` [PATCH 5/5] drivers/serial: Return -ENOMEM on memory allocation Patrick Gefre
2010-08-11 16:23 ` [PATCH 5/5] drivers/serial: Return -ENOMEM on memory allocation failure Julia Lawall
2010-08-11 16:23 ` [PATCH 5/5] drivers/serial: Return -ENOMEM on memory allocation Julia Lawall
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.