Prev: [RFC PATCH 3/5] Btrfs: 3 new ioctls related to hot data features
Next: mmotm 2010-07-27-14-56 uploaded
From: Jiri Slaby on 27 Jul 2010 18:30
On 07/27/2010 08:42 PM, Kulikov Vasiliy wrote:
> 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 <segooon(a)gmail.com>
> ---
> drivers/media/dvb/siano/smscoreapi.c | 6 ++++--
> 1 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/dvb/siano/smscoreapi.c b/drivers/media/dvb/siano/smscoreapi.c
> index 7f2c94a..d93468c 100644
> --- a/drivers/media/dvb/siano/smscoreapi.c
> +++ b/drivers/media/dvb/siano/smscoreapi.c
> @@ -1113,9 +1113,11 @@ struct smscore_buffer_t *smscore_getbuffer(struct smscore_device_t *coredev)
> */
>
> prepare_to_wait(&coredev->buffer_mng_waitq, &wait, TASK_INTERRUPTIBLE);
> -
> - if (list_empty(&coredev->buffers))
> + if (list_empty(&coredev->buffers)) {
> + spin_unlock_irqrestore(&coredev->bufferslock, flags);
> schedule();
> + spin_lock_irqsave(&coredev->bufferslock, flags);
> + }
>
> finish_wait(&coredev->buffer_mng_waitq, &wait);

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?

regards,
--
js
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Richard Zidlicky on 8 Aug 2010 12:20
On Wed, Jul 28, 2010 at 12:24:39AM +0200, Jiri Slaby wrote:

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

> 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.

Richard


--- linux-2.6.34/drivers/media/dvb/siano/smscoreapi.c.rz 2010-06-03 21:58:11.000000000 +0200
+++ linux-2.6.34/drivers/media/dvb/siano/smscoreapi.c 2010-06-07 14:32:06.000000000 +0200
@@ -1100,31 +1100,26 @@
*
* @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);
-
- /* 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);
-
+ 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;
+}
+
+struct smscore_buffer_t *smscore_getbuffer(struct smscore_device_t *coredev)
+{
+ struct smscore_buffer_t *cb = NULL;
+
+ wait_event(coredev->buffer_mng_waitq, (cb = get_entry(coredev)));

return cb;
}


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
 | 
Pages: 1
Prev: [RFC PATCH 3/5] Btrfs: 3 new ioctls related to hot data features
Next: mmotm 2010-07-27-14-56 uploaded