netdev - Re: [PATCH 4/4] phylib: Allow reading and writing a mii bus from atomic context.

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: <20100705.190547.28805625.davem@davemloft.net>
Date:	Mon, 05 Jul 2010 19:05:47 -0700 (PDT)
From:	David Miller <davem@...emloft.net>
To:	richardcochran@...il.com
Cc:	netdev@...r.kernel.org
Subject: Re: [PATCH 4/4] phylib: Allow reading and writing a mii bus from
 atomic context.

From: Richard Cochran <richardcochran@...il.com>
Date: Mon, 5 Jul 2010 07:33:14 +0200

> In order to support hardware time stamping from a PHY, it is necessary to
> read from the PHY while running in_interrupt(). This patch allows a mii
> bus to operate in an atomic context. An mii_bus driver may declare itself
> capable for this mode. Drivers which do not do this will remain with the
> default that bus operations may sleep.
> 
> Before commit 35b5f6b1a82b5c586e0b24c711dc6ba944e88ef1 mii bus
> operations were protected with spin locks. That commit replaced the
> locks with mutexs in order to accommodate i2c buses that need to
> sleep. Thus, this patch restores the original behavior as a run time
> option.
> 
> Signed-off-by: Richard Cochran <richard.cochran@...cron.at>

Ok, I'm not to happy about this one, to be honest.

Conditional locking is always a problem waiting to happen.

Question: What context can you call mdiobus_lock() in?

Answer: You absolutely cannot know.

Therefore every piece of code, except those with special knowledge
of what kind of device is behind the mdiobus object, have to assume
the worst which is that the interface can only be called in sleepable
contexts.

We need to find another way to accomodate this, I really don't want to
see this kind of situation where the locking facility type is
conditional upon the device sitting behind the interface.
--
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