2006-10-07 23:26:42

by Om Narasimhan

[permalink] [raw]
Subject: fixed PCMCIA au1000_generic.c potential crash.

Hi,
Please find the corrected patch.
This patch fixes the following issues in drivers/pcmcia/au1000_generic.c.

1. On the error path, skt would not contain a valid value for the
first iteration (skt is masked by uninitialized automatic variable
skt) which would cause a crash.
2. does not do hw_shutdown() for 0th element of PCMCIA_SOCKET.


Applies cleanly to 2.6.18-rc6, rc7, 2.6.18, and 2.6.19-rc1
Regards,
Om.


drivers/pcmcia/au1000_generic.c | 15 +++++++++------
1 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/pcmcia/au1000_generic.c b/drivers/pcmcia/au1000_generic.c
index d5dd0ce..5387de6 100644
--- a/drivers/pcmcia/au1000_generic.c
+++ b/drivers/pcmcia/au1000_generic.c
@@ -351,6 +351,7 @@ struct skt_dev_info {
int au1x00_pcmcia_socket_probe(struct device *dev, struct
pcmcia_low_level *ops, int first, int nr)
{
struct skt_dev_info *sinfo;
+ struct au1000_pcmcia_socket *skt;
int ret, i;

sinfo = kzalloc(sizeof(struct skt_dev_info), GFP_KERNEL);
@@ -365,7 +366,7 @@ int au1x00_pcmcia_socket_probe(struct de
* Initialise the per-socket structure.
*/
for (i = 0; i < nr; i++) {
- struct au1000_pcmcia_socket *skt = PCMCIA_SOCKET(i);
+ skt = PCMCIA_SOCKET(i);
memset(skt, 0, sizeof(*skt));

skt->socket.resource_ops = &pccard_static_ops;
@@ -438,17 +439,19 @@ #endif
dev_set_drvdata(dev, sinfo);
return 0;

- do {
- struct au1000_pcmcia_socket *skt = PCMCIA_SOCKET(i);
+
+out_err:
+ flush_scheduled_work();
+ ops->hw_shutdown(skt);
+ while (i-- > 0) {
+ skt = PCMCIA_SOCKET(i);

del_timer_sync(&skt->poll_timer);
pcmcia_unregister_socket(&skt->socket);
-out_err:
flush_scheduled_work();
ops->hw_shutdown(skt);

- i--;
- } while (i > 0);
+ }
kfree(sinfo);
out:
return ret;


2006-10-08 20:55:46

by Andrew Morton

[permalink] [raw]
Subject: Re: fixed PCMCIA au1000_generic.c potential crash.

On Sat, 7 Oct 2006 16:26:40 -0700
"Om Narasimhan" <[email protected]> wrote:

> Please find the corrected patch.

The patch is wordwrapped, has spaces replaced by tabs and each version is
subtly different from its predecessor. This confuses me.

Please confirm that the below is the appropriate and final patch, thanks.



From: "Om Narasimhan" <[email protected]>

The previous code did something like,

if (error) goto out_err;
....
do {
struct au1000_pcmcia_socket *skt = PCMCIA_SOCKET(i);
del_timer_sync(&skt->poll_timer);
pcmcia_unregister_socket(&skt->socket);
out_err:
flush_scheduled_work();
ops->hw_shutdown(skt);
i--;
} while (i > 0)
.....


- On the error path, skt would not contain a valid value for the first
iteration (skt is masked by uninitialized automatic skt)

- Does not do hw_shutdown() for 0th element of PCMCIA_SOCKET

Signed-off-by: Om Narasimhan <[email protected]>
Cc: Dominik Brodowski <[email protected]>
Cc: "Yoichi Yuasa" <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
---

drivers/pcmcia/au1000_generic.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)

diff -puN drivers/pcmcia/au1000_generic.c~pcmcia-au1000_generic-fix drivers/pcmcia/au1000_generic.c
--- a/drivers/pcmcia/au1000_generic.c~pcmcia-au1000_generic-fix
+++ a/drivers/pcmcia/au1000_generic.c
@@ -351,6 +351,7 @@ struct skt_dev_info {
int au1x00_pcmcia_socket_probe(struct device *dev, struct pcmcia_low_level *ops, int first, int nr)
{
struct skt_dev_info *sinfo;
+ struct au1000_pcmcia_socket *skt;
int ret, i;

sinfo = kzalloc(sizeof(struct skt_dev_info), GFP_KERNEL);
@@ -365,7 +366,7 @@ int au1x00_pcmcia_socket_probe(struct de
* Initialise the per-socket structure.
*/
for (i = 0; i < nr; i++) {
- struct au1000_pcmcia_socket *skt = PCMCIA_SOCKET(i);
+ skt = PCMCIA_SOCKET(i);
memset(skt, 0, sizeof(*skt));

skt->socket.resource_ops = &pccard_static_ops;
@@ -438,17 +439,19 @@ int au1x00_pcmcia_socket_probe(struct de
dev_set_drvdata(dev, sinfo);
return 0;

- do {
- struct au1000_pcmcia_socket *skt = PCMCIA_SOCKET(i);
+
+out_err:
+ flush_scheduled_work();
+ ops->hw_shutdown(skt);
+ while (i-- > 0) {
+ skt = PCMCIA_SOCKET(i);

del_timer_sync(&skt->poll_timer);
pcmcia_unregister_socket(&skt->socket);
-out_err:
flush_scheduled_work();
ops->hw_shutdown(skt);

- i--;
- } while (i > 0);
+ }
kfree(sinfo);
out:
return ret;
_

2006-10-09 20:01:55

by Om Narasimhan

[permalink] [raw]
Subject: Re: fixed PCMCIA au1000_generic.c potential crash.

On 10/8/06, Andrew Morton <[email protected]> wrote:
> On Sat, 7 Oct 2006 16:26:40 -0700
> "Om Narasimhan" <[email protected]> wrote:
>
> > Please find the corrected patch.
>
> The patch is wordwrapped, has spaces replaced by tabs and each version is
> subtly different from its predecessor. This confuses me.
>
> Please confirm that the below is the appropriate and final patch, thanks.
Yes. it is.
Thanks
Om