netdev - Re: [PATCH 3/3] via-velocity: Fix races on shared interrupts

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:	Thu, 11 Feb 2010 09:05:49 +0100
From:	Simon Kagstrom <simon.kagstrom@...insight.net>
To:	Laurent Chavey <chavey@...gle.com>
Cc:	netdev@...r.kernel.org, davem@...emloft.net, davej@...hat.com,
	ben@...adent.org.uk
Subject: Re: [PATCH 3/3] via-velocity: Fix races on shared interrupts

Hi Laurent!

On Wed, 10 Feb 2010 09:41:59 -0800
Laurent Chavey <chavey@...gle.com> wrote:

> > -       spin_lock(&vptr->lock);
> > +       /* Check if the lock is taken, and if so ignore the interrupt. This
> > +        * can happen with shared interrupts, where the other device can
> > +        * interrupt during velocity_poll (where the lock is held).
> > +        *
> > +        * With spinlock debugging active on a uniprocessor, this will give
> > +        * a warning which can safely be ignored.
> > +        */
> > +       if (!spin_trylock(&vptr->lock))
> > +               return IRQ_NONE;
> 
> does the thread handling the interrupts check that an new
> interrupts was received while it was servicing a previous one ?
> wondering if there is a potential for an event that generates the interrupt
> to be missed.

I should say that this particular part of the patch was reworked in
version 2, which David took in here:
  
  http://git.kernel.org/?p=linux/kernel/git/davem/net-2.6.git;a=commitdiff;h=3f2e8d9f13246382fbda6f03178eef867a9bfbe2

Anyway, velocity_poll will try to empty all events within it's budget
from the device and is executing with the device interrupt turned off
(and now also with the local processor interrupts off). If something
would be posted when it's exiting, that's fine since it either

  1) Consumed it's entire budget, in which case it will stay in the
  polling mode anyway

or,

  2) Didn't consume the budget and will then turn on the interrupt
  again and get the new event promptly.


You are right about the code above though, that one is racy as David
explained here:

  http://permalink.gmane.org/gmane.linux.network/151578

// Simon
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html