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

[Andrew Morton <akpm@osdl.org>]

Corey Minyard <minyard@acm.org> wrote:
>
> >- 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?

Ah, you are of course correct.  Consider me thwapped.