Re: [PATCH] pcmcia driver model support [4/5]

[Adam Belay <ambx1@neo.rr.com>]

On Fri, Aug 06, 2004 at 11:43:20AM +0100, Russell King wrote:
> On Thu, Aug 05, 2004 at 10:28:20PM +0000, Adam Belay wrote:
> > It is not safe to use the skt_sem in pcmcia_validate_mem.  This patch
> > fixes a real world bug, and without it many systems will fail to shutdown
> > properly.
> 
> However, we need to take this semaphore here to prevent the socket state
> changing.  It sounds from your description that we're hitting yet another
> stupid recursion bug in PCMCIA...

It's worth noting that we don't hold skt_sem in pcmcia_get_first_tuple (and
possibly others), but we probably should be.  This may have been to prevent
recursion bugs.

> 
> It sounds like we shouldn't be holding skt_sem when we wait for userspace
> to reply to the ejection request.

The situation is rather complicated.  pcmcia_eject_card itself has to hold
skt_sem to ensure the socket state remains correct.  We could always release
the semaphore while sending the event, and then grab it again.  Of course we
would have to check if the socket is still present a second time in the same
function.  How does this look (untested)?

--- a/drivers/pcmcia/cs.c	2004-08-05 21:28:48.000000000 +0000
+++ b/drivers/pcmcia/cs.c	2004-08-06 09:52:47.000000000 +0000
@@ -2056,11 +2056,17 @@
 			break;
 		}
 
+		up(&skt->skt_sem);
 		ret = send_event(skt, CS_EVENT_EJECTION_REQUEST, CS_EVENT_PRI_LOW);
 		if (ret != 0) {
 			ret = -EINVAL;
 			break;
 		}
+		down(&skt->sem);
+		if (!(skt->state & SOCKET_PRESENT)) {
+			ret = -ENODEV;
+			break;
+		}
 
 		socket_remove(skt);
 		ret = 0;