| 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
| ||
|
Message-ID: <20220613073208.anak24kpffnngube@pengutronix.de>
Date: Mon, 13 Jun 2022 09:32:08 +0200
From: Marc Kleine-Budde <mkl@...gutronix.de>
To: Dario Binacchi <dario.binacchi@...rulasolutions.com>
Cc: linux-kernel@...r.kernel.org, michael@...rulasolutions.com,
Amarula patchwork <linux-amarula@...rulasolutions.com>,
Oliver Hartkopp <socketcan@...tkopp.net>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Jakub Kicinski <kuba@...nel.org>,
Jiri Slaby <jirislaby@...nel.org>,
Paolo Abeni <pabeni@...hat.com>,
Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
Vincent Mailhol <mailhol.vincent@...adoo.fr>,
Wolfgang Grandegger <wg@...ndegger.com>,
linux-can@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [PATCH v3 12/13] can: slcan: extend the protocol with error info
On 12.06.2022 23:39:26, Dario Binacchi wrote:
> It extends the protocol to receive the adapter CAN communication errors
> and forward them to the netdev upper levels.
>
> Signed-off-by: Dario Binacchi <dario.binacchi@...rulasolutions.com>
>
> ---
>
> (no changes since v2)
>
> Changes in v2:
> - Protect decoding against the case the len value is longer than the
> received data.
Where is that check?
> - Continue error handling even if no skb can be allocated.
>
> drivers/net/can/slcan/slcan-core.c | 130 ++++++++++++++++++++++++++++-
> 1 file changed, 129 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/can/slcan/slcan-core.c b/drivers/net/can/slcan/slcan-core.c
> index 3df35ae8f040..48077edb9497 100644
> --- a/drivers/net/can/slcan/slcan-core.c
> +++ b/drivers/net/can/slcan/slcan-core.c
> @@ -175,8 +175,118 @@ int slcan_enable_err_rst_on_open(struct net_device *ndev, bool on)
> * STANDARD SLCAN DECAPSULATION *
> ************************************************************************/
>
> +static void slc_bump_err(struct slcan *sl)
> +{
> + struct net_device *dev = sl->dev;
> + struct sk_buff *skb;
> + struct can_frame *cf;
> + char *cmd = sl->rbuff;
> + bool rx_errors = false, tx_errors = false;
> + int i, len;
> +
> + if (*cmd != 'e')
> + return;
This has already been checked in the caller, right?
> +
> + cmd += SLC_CMD_LEN;
> + /* get len from sanitized ASCII value */
> + len = *cmd++;
> + if (len >= '0' && len < '9')
> + len -= '0';
> + else
> + return;
> +
> + skb = alloc_can_err_skb(dev, &cf);
> +
> + if (skb)
> + cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
> +
> + for (i = 0; i < len; i++, cmd++) {
> + switch (*cmd) {
> + case 'a':
> + netdev_dbg(dev, "ACK error\n");
> + tx_errors = true;
Nitpick:
Please decide if you want to set tx/tx_errors here and increment at the
end of the function....or.....
> + if (skb) {
> + cf->can_id |= CAN_ERR_ACK;
> + cf->data[3] = CAN_ERR_PROT_LOC_ACK;
> + }
> +
> + break;
> + case 'b':
> + netdev_dbg(dev, "Bit0 error\n");
> + tx_errors = true;
> + if (skb)
> + cf->data[2] |= CAN_ERR_PROT_BIT0;
> +
> + break;
> + case 'B':
> + netdev_dbg(dev, "Bit1 error\n");
> + tx_errors = true;
> + if (skb)
> + cf->data[2] |= CAN_ERR_PROT_BIT1;
> +
> + break;
> + case 'c':
> + netdev_dbg(dev, "CRC error\n");
> + rx_errors = true;
> + if (skb) {
> + cf->data[2] |= CAN_ERR_PROT_BIT;
> + cf->data[3] = CAN_ERR_PROT_LOC_CRC_SEQ;
> + }
> +
> + break;
> + case 'f':
> + netdev_dbg(dev, "Form Error\n");
> + rx_errors = true;
> + if (skb)
> + cf->data[2] |= CAN_ERR_PROT_FORM;
> +
> + break;
> + case 'o':
> + netdev_dbg(dev, "Rx overrun error\n");
> + dev->stats.rx_over_errors++;
> + dev->stats.rx_errors++;
....if you want to increment in the case.
> + if (skb) {
> + cf->can_id |= CAN_ERR_CRTL;
> + cf->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;
> + }
> +
> + break;
> + case 'O':
> + netdev_dbg(dev, "Tx overrun error\n");
> + dev->stats.tx_errors++;
> + if (skb) {
> + cf->can_id |= CAN_ERR_CRTL;
> + cf->data[1] = CAN_ERR_CRTL_TX_OVERFLOW;
> + }
> +
> + break;
> + case 's':
> + netdev_dbg(dev, "Stuff error\n");
> + rx_errors = true;
> + if (skb)
> + cf->data[2] |= CAN_ERR_PROT_STUFF;
> +
> + break;
> + default:
> + if (skb)
> + dev_kfree_skb(skb);
> +
> + return;
> + }
> + }
> +
> + if (rx_errors)
> + dev->stats.rx_errors++;
> +
> + if (tx_errors)
> + dev->stats.tx_errors++;
> +
> + if (skb)
> + netif_rx(skb);
> +}
> +
> /* Send one completely decapsulated can_frame to the network layer */
> -static void slc_bump(struct slcan *sl)
> +static void slc_bump_frame(struct slcan *sl)
> {
> struct sk_buff *skb;
> struct can_frame *cf;
> @@ -255,6 +365,24 @@ static void slc_bump(struct slcan *sl)
> dev_kfree_skb(skb);
> }
>
> +static void slc_bump(struct slcan *sl)
> +{
> + switch (sl->rbuff[0]) {
> + case 'r':
> + fallthrough;
> + case 't':
> + fallthrough;
> + case 'R':
> + fallthrough;
> + case 'T':
> + return slc_bump_frame(sl);
> + case 'e':
> + return slc_bump_err(sl);
> + default:
> + return;
> + }
> +}
> +
> /* parse tty input stream */
> static void slcan_unesc(struct slcan *sl, unsigned char s)
> {
> --
> 2.32.0
>
>
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung West/Dortmund | Phone: +49-231-2826-924 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)
Powered by blists - more mailing lists