netdev - Re: [PATCH v3 0/9] atm: propagate atm_dev signal carrier to LOWER

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:	Wed, 07 Jul 2010 15:07:41 -0700 (PDT)
From:	David Miller <davem@...emloft.net>
To:	karl@...amoto.org
Cc:	linux-atm-general@...ts.sourceforge.net, netdev@...r.kernel.org,
	chas@....nrl.navy.mil
Subject: Re: [PATCH v3 0/9] atm: propagate atm_dev signal carrier to
 LOWER_UP of netdevice

From: Karl Hiramoto <karl@...amoto.org>
Date: Wed,  7 Jul 2010 10:50:27 +0200

> Changes from v2:
>  * use atomic instead of blocking notifier
>  * use read_lock_irq() instead of read_lock() in atm/br2684
>  * clean up comments
>  * remove unused variable.  I feel really bad about missing that last time.
>  
> Changes from v1:
> Use atm_dev notifier chain  instead of callback function pointer in struct vcc.
> In drivers/usb/atm call atm_dev_signal_change().
> 
> In userspace it's helpful to know if a network device has a carrier signal.
> Often it is monitored via netlink.  This patchset allows a way for the
> struct atm_dev drivers to pass carrier on/off to the netdevice.
> 
> For DSL, carrier is on when the line has reached showtime state.
> 
> Currently this patchset only propagates the changes to br2684 vccs,
> as this is the only type of hardware I have to test.
> 
> If you prefer git you can pull from:
> git://github.com/karlhiramoto/linux-2.6.git atm-v3

I think the locking still needs another tweak.

By using read_lock_irq() you are assuming that you are invoked
from a context where irqs are disabled.

That's not necessarily the case, in fact some of your notifier call
sites in the drivers are in interrupt handlers where interrupts may or
may not be disabled.

So you'll likely need to use read_lock_irqsave() and read_lock_irqrestore().

Next, please format comments:

/* Like
 * this.
 */

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