Re: [PATCH v3] ethtool: avoiding integer overflow in ethtool_phys_id()
From: Alexander Lobakin <hidden>
Date: 2022-11-22 13:39:22
Also in:
lkml
From: Maxim Korotkov <redacted> Date: Tue, 22 Nov 2022 15:29:01 +0300
The value of an arithmetic expression "n * id.data" is subject to possible overflow due to a failure to cast operands to a larger data type before performing arithmetic. Used macro for multiplication instead operator for avoiding overflow. Found by Linux Verification Center (linuxtesting.org) with SVACE. Signed-off-by: Maxim Korotkov <redacted>
Reviewed-by: Alexander Lobakin <redacted>
quoted hunk ↗ jump to hunk
--- net/ethtool/ioctl.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c index 6a7308de192d..6b59e7a1c906 100644 --- a/net/ethtool/ioctl.c +++ b/net/ethtool/ioctl.c@@ -2007,7 +2007,8 @@ static int ethtool_phys_id(struct net_device *dev, void __user *useraddr) } else { /* Driver expects to be called at twice the frequency in rc */ int n = rc * 2, interval = HZ / n; - u64 count = n * id.data, i = 0; + u64 count = mul_u32_u32(n, id.data); + u64 i = 0; do { rtnl_lock();-- 2.17.1
Some notes to the process, not the code: 1) I asked to add my Reviewed-by to v3 in the previous thread, it's mandatory for authors to pick-up all the tags before publishing a new revision; 2) when sending v2, v3, ... vN, please have a changelog in the commit message or right below those '---' after your SoB, that makes it easier to review. Thanks, Olek