linux-kernel - Re: [PATCH] can: kvaser_pciefd: refine error prone echo_skb

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: <87wma1nf7c.fsf@kvaser.com>
Date: Wed, 28 May 2025 13:32:23 +0200
From: Axel Forsman <axfo@...ser.com>
To: Fedor Pchelkin <pchelkin@...ras.ru>, Marc Kleine-Budde
 <mkl@...gutronix.de>, Vincent Mailhol <mailhol.vincent@...adoo.fr>
Cc: Fedor Pchelkin <pchelkin@...ras.ru>, Jimmy Assarsson <extja@...ser.com>,
 linux-can@...r.kernel.org, linux-kernel@...r.kernel.org,
 lvc-project@...uxtesting.org, stable@...r.kernel.org
Subject: Re: [PATCH] can: kvaser_pciefd: refine error prone echo_skb_max
 handling logic

Thanks for finding and fixing this bug.

Fedor Pchelkin <pchelkin@...ras.ru> writes:

> Actually the trick with rounding up allows to calculate seq numbers
> efficiently, avoiding a more consuming 'mod' operation used in the
> current patch.

Indeed, that was the intention.

> So another approach to fix the problem would be to precompute the rounded
> up value of echo_skb_max and pass it to alloc_candev() making the size of
> the underlying echo_skb[] sufficient.

I believe that is preferable---if memory usage is a concern
KVASER_PCIEFD_CAN_TX_MAX_COUNT could be lowered by one.
Something like the following:

diff --git a/drivers/net/can/kvaser_pciefd.c b/drivers/net/can/kvaser_pciefd.c
index f6921368cd14..0071a51ce2c1 100644
--- a/drivers/net/can/kvaser_pciefd.c
+++ b/drivers/net/can/kvaser_pciefd.c
@@ -966,7 +966,7 @@ static int kvaser_pciefd_setup_can_ctrls(struct kvaser_pciefd *pcie)
                u32 status, tx_nr_packets_max;

                netdev = alloc_candev(sizeof(struct kvaser_pciefd_can),
-                                     KVASER_PCIEFD_CAN_TX_MAX_COUNT);
+                                     roundup_pow_of_two(KVASER_PCIEFD_CAN_TX_MAX_COUNT));
                if (!netdev)
                        return -ENOMEM;

@@ -995,7 +995,6 @@ static int kvaser_pciefd_setup_can_ctrls(struct kvaser_pciefd *pcie)
                can->tx_max_count = min(KVASER_PCIEFD_CAN_TX_MAX_COUNT, tx_nr_packets_max - 1);

                can->can.clock.freq = pcie->freq;
-               can->can.echo_skb_max = roundup_pow_of_two(can->tx_max_count);
                spin_lock_init(&can->lock);

                can->can.bittiming_const = &kvaser_pciefd_bittiming_const;


/Axel Forsman