Re: [PATCH] IPMI driver updates, part 1b

[Corey Minyard <minyard@acm.org>]

Andrew Morton wrote:

>Corey Minyard <minyard@acm.org> wrote:
>  
>
>>It helps to actually attach the code.
>>    
>>
>
>Got there eventually.
>
>Patches seem reasonable, thanks.  I'm not sure how to judge the suitability
>of the socket interface but it only touches your code..
>
Actually, since you already have it in, just leave it in.  It's easy to 
pull out.

>
>- There's a locking bug in ipmi_recvmsg(): it can unlock i_lock when it
>  isn't held.   I added this:
>
>diff -puN net/ipmi/af_ipmi.c~af_ipmi-locking-fix net/ipmi/af_ipmi.c
>--- 25/net/ipmi/af_ipmi.c~af_ipmi-locking-fix	Tue Feb 24 16:56:36 2004
>+++ 25-akpm/net/ipmi/af_ipmi.c	Tue Feb 24 16:57:00 2004
>@@ -336,6 +336,7 @@ static int ipmi_recvmsg(struct kiocb *io
> 		}
> 
> 		timeo = ipmi_wait_for_queue(i, timeo);
>+		spin_lock_irqsave(&i->lock, flags);
> 	}
> 
> 	rcvmsg = list_entry(i->msg_list.next, struct ipmi_recv_msg, link);
>
>
> which may or may not be correct.
>
Actually, I believe the code is correct, and your change will break it.  
This is in a "while (1)" loop, and the only way to get out of this loop 
is to return with the lock not held or to break out of the loop with the 
lock held (and later code will unlock it).  Am I correct here?

-Corey