dvb: siano: free spinlock before schedule()

Calling schedule() holding spinlock with disables irqs is improper. As spinlock protects list coredev->buffers, it can be unlocked untill wakeup. This bug was introduced in a9349315f65cd6a16e8fab1f6cf0fd40f379c4db.

Signed-off-by Kulikov Vasiliy <[email protected]>

---
 drivers/media/dvb/siano/smscoreapi.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

drivers/media/dvb/siano/smscoreapi.c +0 -0

index 7f2c94a..d93468c 100644
@@ -1113,9 +1113,11 @@ struct smscore_buffer_t *smscore_getbuffer(struct smscore_device_t *coredev)
 	 */
 

There is a better fix (which fixes the potential NULL dereference):
http://lkml.org/lkml/2010/6/7/175

Richard, could you address the comments there and resend?

sorry for seeing this so late, was flooded with email lately.

Quoted 4 lines

There is a better fix (which fixes the potential NULL dereference):
http://lkml.org/lkml/2010/6/7/175

Richard, could you address the comments there and resend?

I am running this patch since many weeks (after fixing the compile error obviously). Did not implement your beautification suggestion yet, was doing all kinds of experiments with IR and had plenty of unrelated issues.

Em 08-08-2010 13:10, Richard Zidlicky escreveu:

Quoted 7 lines · Jiri Slaby

sorry for seeing this so late, was flooded with email lately.

Quoted 2 lines

There is a better fix (which fixes the potential NULL dereference):
http://lkml.org/lkml/2010/6/7/175

Quoted 5 lines

Quoted 1 line

Richard, could you address the comments there and resend?

I am running this patch since many weeks (after fixing the compile error obviously). Did not implement your beautification suggestion yet, was doing all kinds of experiments with IR and had plenty of unrelated issues.

This patch seems a way better than the previous patch. I've rebased it against the current tree (and fixed the identation).

The only missing issue on it is the lack of your Signed-off-by. Richard, could you please send it to us?

---
 drivers/media/dvb/siano/smscoreapi.c |   31 +++++++++++++------------------
 1 file changed, 13 insertions(+), 18 deletions(-)

drivers/media/dvb/siano/smscoreapi.c +13 -18

@@ -1098,31 +1098,26 @@ EXPORT_SYMBOL_GPL(smscore_onresponse);
  *
  * @return pointer to descriptor on success, NULL on error.
  */
-struct smscore_buffer_t *smscore_getbuffer(struct smscore_device_t *coredev)
+
+struct smscore_buffer_t *get_entry(struct smscore_device_t *coredev)
 {
 	struct smscore_buffer_t *cb = NULL;
 	unsigned long flags;
 
-	DEFINE_WAIT(wait);
-
 	spin_lock_irqsave(&coredev->bufferslock, flags);
+	if (!list_empty(&coredev->buffers)) {
+		cb = (struct smscore_buffer_t *) coredev->buffers.next;
+		list_del(&cb->entry);
+	}
+	spin_unlock_irqrestore(&coredev->bufferslock, flags);
+	return cb;
+}
 
-	/* This function must return a valid buffer, since the buffer list is
-	 * finite, we check that there is an available buffer, if not, we wait
-	 * until such buffer become available.
-	 */
-
-	prepare_to_wait(&coredev->buffer_mng_waitq, &wait, TASK_INTERRUPTIBLE);
-
-	if (list_empty(&coredev->buffers))
-		schedule();
-
-	finish_wait(&coredev->buffer_mng_waitq, &wait);
-
-	cb = (struct smscore_buffer_t *) coredev->buffers.next;
-	list_del(&cb->entry);
+struct smscore_buffer_t *smscore_getbuffer(struct smscore_device_t *coredev)
+{
+	struct smscore_buffer_t *cb = NULL;
 
-	spin_unlock_irqrestore(&coredev->bufferslock, flags);
+	wait_event(coredev->buffer_mng_waitq, (cb = get_entry(coredev)));
 
 	return cb;
 }