2006-09-18 00:54:25

by Om Narasimhan

[permalink] [raw]
Subject: potential crash fix : drivers/pcmcia/au1000_generic

Tested by compiling.

I have not subscribed to pcmcia list. Please cc me any comments.

Signed off by Om Narasimhan <[email protected]>

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

diff --git a/drivers/pcmcia/au1000_generic.c b/drivers/pcmcia/au1000_generic.c
index d5dd0ce..9a87a87 100644
--- a/drivers/pcmcia/au1000_generic.c
+++ b/drivers/pcmcia/au1000_generic.c
@@ -4,7 +4,7 @@
*
* Copyright 2001-2003 MontaVista Software Inc.
* Author: MontaVista Software, Inc.
- * [email protected] or [email protected]
+ * [email protected] or [email protected]
*
* Copyright 2004 Pete Popov, Embedded Alley Solutions, Inc.
* Updated the driver to 2.6. Followed the sa11xx API and largely
@@ -438,17 +438,16 @@ #endif
dev_set_drvdata(dev, sinfo);
return 0;

- do {
+out_err:
+ flush_scheduled_work();
+ ops->hw_shutdown(skt);
+ while (i-- > 0) {
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);
+ }
kfree(sinfo);
out:
return ret;


2006-09-18 11:08:13

by Matthew Wilcox

[permalink] [raw]
Subject: Re: [KJ] potential crash fix : drivers/pcmcia/au1000_generic

On Sun, Sep 17, 2006 at 05:54:17PM -0700, Om Narasimhan wrote:
> Tested by compiling.
>
> I have not subscribed to pcmcia list. Please cc me any comments.

You also haven't described what the 'potential crach' is that you're
fixing.

2006-09-18 22:54:19

by Om Narasimhan

[permalink] [raw]
Subject: Re: [KJ] potential crash fix : drivers/pcmcia/au1000_generic

On 9/18/06, Matthew Wilcox <[email protected]> wrote:
> On Sun, Sep 17, 2006 at 05:54:17PM -0700, Om Narasimhan wrote:
> > Tested by compiling.
> >
> > I have not subscribed to pcmcia list. Please cc me any comments.
>
> You also haven't described what the 'potential crach' is that you're
> fixing.
Okay, I should have documented that.
Here is the explanation.
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)
.....

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

Does it sound good?

Regards,
Om.