netdev - Re: [PATCH 1/4] net: phy: balance disable/enable irq on change

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] [thread-next>] [day] [month] [year] [list]

Message-Id: <20101223.112450.246547841.davem@davemloft.net>
Date:	Thu, 23 Dec 2010 11:24:50 -0800 (PST)
From:	David Miller <davem@...emloft.net>
To:	jhautbois@...il.com
Cc:	richard.cochran@...cron.at, shemminger@...tta.com, tj@...nel.org,
	randy.dunlap@...cle.com, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/4] net: phy: balance disable/enable irq on change

From: Jean-Michel Hautbois <jhautbois@...il.com>
Date: Sun, 19 Dec 2010 11:58:48 +0100

> When phy interface changes its status, it calls phy_change() function.
> This function calls the interrupt disabling functions for the driver
> registered, but if this driver doesn't implement it, there is no IRQ
> disabling. After doing the work, we call enable_irq and not the
> respective driver function. This fixes it, as it could lead to an
> unbalanced IRQ. Error code changed to EOPNOTSUPP.
> 
> Signed-off-by: Jean-Michel Hautbois <jhautbois@...il.com>

This is completely bogus.

First of all, there are 5 call sites for phy_change_interrupt() but
you've only implemented the new semantics for two of those.

Therefore, if we even wanted this, we should implement the behavior in
phy_change_interrupt() itself instead of duplicating the logic at
each and every call site.

But we don't want this.

It's not appropriate at all.  If a device lacks a way to turn
interrupt off and on, using disable_irq() and enable_irq() is not
necessarily correct.

If the interrupt line is shared, for example, this will break
everything.

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