RxDMAIntCtrl encodes rx_coalesce in the low 16 bits
and rx_timeout in the high 16 bits. If either value exceeds
the field width, the current code may truncate the value and/or
corrupt adjacent bits when programming the register.

Mask both values to 16 bits and cast to u32 before shifting
so only the intended fields are written.

Found by inspection.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Tested-on: D-Link DGE-550T Rev-A3
Signed-off-by: Yeounsu Moon <yyyynoom@gmail.com>
---
 drivers/net/ethernet/dlink/dl2k.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/dlink/dl2k.c b/drivers/net/ethernet/dlink/dl2k.c
index 846d58c769ea..f6ec0e4689f7 100644
--- a/drivers/net/ethernet/dlink/dl2k.c
+++ b/drivers/net/ethernet/dlink/dl2k.c
@@ -590,7 +590,8 @@ static void rio_hw_init(struct net_device *dev)
 
 	set_multicast (dev);
 	if (np->coalesce) {
-		dw32(RxDMAIntCtrl, np->rx_coalesce | np->rx_timeout << 16);
+		dw32(RxDMAIntCtrl, ((u32)np->rx_coalesce & 0x0000ffff) |
+				   (((u32)np->rx_timeout & 0x0000ffff) << 16));
 	}
 	/* Set RIO to poll every N*320nsec. */
 	dw8(RxDMAPollPeriod, 0x20);
-- 
2.52.0

On Tue, Dec 23, 2025 at 09:10:06AM +0900, Yeounsu Moon wrote:
> RxDMAIntCtrl encodes rx_coalesce in the low 16 bits
> and rx_timeout in the high 16 bits. If either value exceeds
> the field width, the current code may truncate the value and/or
> corrupt adjacent bits when programming the register.
> 
> Mask both values to 16 bits and cast to u32 before shifting
> so only the intended fields are written.

It would be better to do range checks in rio_probe1() and call
netdev_err() and return -EINVAL?

Anybody trying to use very large values then gets an error message
rather than it working, but not as expected.

	Andrew

On 12/23/25 10:43 AM, Andrew Lunn wrote:
> On Tue, Dec 23, 2025 at 09:10:06AM +0900, Yeounsu Moon wrote:
>> RxDMAIntCtrl encodes rx_coalesce in the low 16 bits
>> and rx_timeout in the high 16 bits. If either value exceeds
>> the field width, the current code may truncate the value and/or
>> corrupt adjacent bits when programming the register.
>>
>> Mask both values to 16 bits and cast to u32 before shifting
>> so only the intended fields are written.
> 
> It would be better to do range checks in rio_probe1() and call
> netdev_err() and return -EINVAL?
> 
> Anybody trying to use very large values then gets an error message
> rather than it working, but not as expected.

I'm not sure we can do such change: any eventual user with bad setting
will get a broken setup after kernel update. I think we should avoid
such regression, and use something similar to this patch.

@Yeounsu: the type cast in the current patch is not needed, please drop
it, thanks!

Paolo

On Tue Dec 30, 2025 at 7:57 PM KST, Paolo Abeni wrote:
> I'm not sure we can do such change: any eventual user with bad setting
> will get a broken setup after kernel update. I think we should avoid
> such regression, and use something similar to this patch.
>
> @Yeounsu: the type cast in the current patch is not needed, please drop
> it, thanks!
>
> Paolo
Thank you for the review. I'll send v2 patch as suggested.

Separately, I agree with Andrew that it would be useful to alert users
when an invalid value is provided. Would it be OK to send a net-next
follow-up that warns (via netdev_warn()) when rx_coalesce or rx_timeout
exceeds 16 bits, while keeping the current behavior (no -EINVAL)?

    Yeounsu Moon

Hi Andrew,

Sorry for the late reply. I recently started a new job and have been
busy.

On Tue Dec 23, 2025 at 6:43 PM KST, Andrew Lunn wrote:
>
> It would be better to do range checks in rio_probe1() and call
> netdev_err() and return -EINVAL?
>
> Anybody trying to use very large values then gets an error message
> rather than it working, but not as expected.
>
I was planning to add the range checks in rio_probe1() in the next merge
window to keep this patch small, but I agree it's better to include them now.
I'll send a v2 with the checks added.

Thanks for the review.

    Yeounsu Moon