linux-kernel - Re: schedule inside spin_lock

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4C028336.8030704@gmail.com>
Date:	Sun, 30 May 2010 17:24:38 +0200
From:	Jiri Slaby <jirislaby@...il.com>
To:	Richard Zidlicky <rz@...ux-m68k.org>
CC:	linux-kernel@...r.kernel.org, linux-media@...r.kernel.org
Subject: Re: schedule inside spin_lock_irqsave?

On 05/30/2010 04:52 PM, Richard Zidlicky wrote:
> Hi,
> 
> came across following snippet of code (2.6.34:drivers/media/dvb/siano/smscoreapi.c) and 
> since prepare_to_wait is new for me I am wondering if this is can work?
> 
> struct smscore_buffer_t *smscore_getbuffer(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);
> 
> 	cb = (struct smscore_buffer_t *) coredev->buffers.next;
> 	list_del(&cb->entry);
> 
> 	spin_unlock_irqrestore(&coredev->bufferslock, flags);


Yep, that's a double bug.
1) If the waiting is interrupted, it will die because the list is still
empty.
2) If there is no entry in the list, it will deadlock at least on UP.
This should be
wait_event(&coredev->buffer_mng_waitq, cb = get_entry());
with get_entry like:
struct smscore_buffer_t *get_entry(void)
{
  struct smscore_buffer_t *cb = NULL;
  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;
}

regards,
-- 
js
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/