Skip to content

Commit 4ec7329

Browse files
Russell King (Oracle)kuba-moo
authored andcommitted
net: phylib: fix phy_read*_poll_timeout()
Dan Carpenter reported a signedness bug in genphy_loopback(). Andrew reports that: "It is common to get this wrong in general with PHY drivers. Dan regularly posts fixes like this soon after a PHY driver patch it merged. I really wish we could somehow get the compiler to warn when the result from phy_read() is stored into a unsigned type. It would save Dan a lot of work." Let's make phy_read*_poll_timeout() immune to further issues when "val" is an unsigned type by storing the read function's result in a signed int as well as "val", and using the signed variable both to check for an error and for propagating that error to the caller. The advantage of this method is we don't change where the cast from the signed return code to the user's variable occurs - so users will see no change. Previously Heiner changed phy_read_poll_timeout() to check for an error before evaluating the user supplied condition, but didn't update phy_read_mmd_poll_timeout(). Make that change there too. Link: https://lore.kernel.org/r/d7bb312e-2428-45f6-b9b3-59ba544e8b94@kili.mountain Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> Link: https://lore.kernel.org/r/E1q4kX6-00BNuM-Mx@rmk-PC.armlinux.org.uk Signed-off-by: Jakub Kicinski <kuba@kernel.org>
1 parent 7fa217d commit 4ec7329

File tree

1 file changed

+10
-6
lines changed

1 file changed

+10
-6
lines changed

include/linux/phy.h

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1206,10 +1206,12 @@ static inline int phy_read(struct phy_device *phydev, u32 regnum)
12061206
#define phy_read_poll_timeout(phydev, regnum, val, cond, sleep_us, \
12071207
timeout_us, sleep_before_read) \
12081208
({ \
1209-
int __ret = read_poll_timeout(phy_read, val, val < 0 || (cond), \
1209+
int __ret, __val; \
1210+
__ret = read_poll_timeout(__val = phy_read, val, \
1211+
__val < 0 || (cond), \
12101212
sleep_us, timeout_us, sleep_before_read, phydev, regnum); \
1211-
if (val < 0) \
1212-
__ret = val; \
1213+
if (__val < 0) \
1214+
__ret = __val; \
12131215
if (__ret) \
12141216
phydev_err(phydev, "%s failed: %d\n", __func__, __ret); \
12151217
__ret; \
@@ -1302,11 +1304,13 @@ int phy_read_mmd(struct phy_device *phydev, int devad, u32 regnum);
13021304
#define phy_read_mmd_poll_timeout(phydev, devaddr, regnum, val, cond, \
13031305
sleep_us, timeout_us, sleep_before_read) \
13041306
({ \
1305-
int __ret = read_poll_timeout(phy_read_mmd, val, (cond) || val < 0, \
1307+
int __ret, __val; \
1308+
__ret = read_poll_timeout(__val = phy_read_mmd, val, \
1309+
__val < 0 || (cond), \
13061310
sleep_us, timeout_us, sleep_before_read, \
13071311
phydev, devaddr, regnum); \
1308-
if (val < 0) \
1309-
__ret = val; \
1312+
if (__val < 0) \
1313+
__ret = __val; \
13101314
if (__ret) \
13111315
phydev_err(phydev, "%s failed: %d\n", __func__, __ret); \
13121316
__ret; \

0 commit comments

Comments
 (0)