From 3ea566422cbde9610c2734980d1286ab681bb40e Mon Sep 17 00:00:00 2001 From: Oliver Hartkopp Date: Wed, 16 Mar 2022 17:42:56 +0100 Subject: [PATCH 1/5] can: isotp: sanitize CAN ID checks in isotp_bind() Syzbot created an environment that lead to a state machine status that can not be reached with a compliant CAN ID address configuration. The provided address information consisted of CAN ID 0x6000001 and 0xC28001 which both boil down to 11 bit CAN IDs 0x001 in sending and receiving. Sanitize the SFF/EFF CAN ID values before performing the address checks. Fixes: e057dd3fc20f ("can: add ISO 15765-2:2016 transport protocol") Link: https://lore.kernel.org/all/20220316164258.54155-1-socketcan@hartkopp.net Reported-by: syzbot+2339c27f5c66c652843e@syzkaller.appspotmail.com Signed-off-by: Oliver Hartkopp Signed-off-by: Marc Kleine-Budde --- net/can/isotp.c | 41 +++++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/net/can/isotp.c b/net/can/isotp.c index d4c0b4704987..1662103ce125 100644 --- a/net/can/isotp.c +++ b/net/can/isotp.c @@ -1148,6 +1148,7 @@ static int isotp_bind(struct socket *sock, struct sockaddr *uaddr, int len) struct net *net = sock_net(sk); int ifindex; struct net_device *dev; + canid_t tx_id, rx_id; int err = 0; int notify_enetdown = 0; int do_rx_reg = 1; @@ -1155,8 +1156,18 @@ static int isotp_bind(struct socket *sock, struct sockaddr *uaddr, int len) if (len < ISOTP_MIN_NAMELEN) return -EINVAL; - if (addr->can_addr.tp.tx_id & (CAN_ERR_FLAG | CAN_RTR_FLAG)) - return -EADDRNOTAVAIL; + /* sanitize tx/rx CAN identifiers */ + tx_id = addr->can_addr.tp.tx_id; + if (tx_id & CAN_EFF_FLAG) + tx_id &= (CAN_EFF_FLAG | CAN_EFF_MASK); + else + tx_id &= CAN_SFF_MASK; + + rx_id = addr->can_addr.tp.rx_id; + if (rx_id & CAN_EFF_FLAG) + rx_id &= (CAN_EFF_FLAG | CAN_EFF_MASK); + else + rx_id &= CAN_SFF_MASK; if (!addr->can_ifindex) return -ENODEV; @@ -1168,21 +1179,13 @@ static int isotp_bind(struct socket *sock, struct sockaddr *uaddr, int len) do_rx_reg = 0; /* do not validate rx address for functional addressing */ - if (do_rx_reg) { - if (addr->can_addr.tp.rx_id == addr->can_addr.tp.tx_id) { - err = -EADDRNOTAVAIL; - goto out; - } - - if (addr->can_addr.tp.rx_id & (CAN_ERR_FLAG | CAN_RTR_FLAG)) { - err = -EADDRNOTAVAIL; - goto out; - } + if (do_rx_reg && rx_id == tx_id) { + err = -EADDRNOTAVAIL; + goto out; } if (so->bound && addr->can_ifindex == so->ifindex && - addr->can_addr.tp.rx_id == so->rxid && - addr->can_addr.tp.tx_id == so->txid) + rx_id == so->rxid && tx_id == so->txid) goto out; dev = dev_get_by_index(net, addr->can_ifindex); @@ -1206,16 +1209,14 @@ static int isotp_bind(struct socket *sock, struct sockaddr *uaddr, int len) ifindex = dev->ifindex; if (do_rx_reg) { - can_rx_register(net, dev, addr->can_addr.tp.rx_id, - SINGLE_MASK(addr->can_addr.tp.rx_id), + can_rx_register(net, dev, rx_id, SINGLE_MASK(rx_id), isotp_rcv, sk, "isotp", sk); /* no consecutive frame echo skb in flight */ so->cfecho = 0; /* register for echo skb's */ - can_rx_register(net, dev, addr->can_addr.tp.tx_id, - SINGLE_MASK(addr->can_addr.tp.tx_id), + can_rx_register(net, dev, tx_id, SINGLE_MASK(tx_id), isotp_rcv_echo, sk, "isotpe", sk); } @@ -1239,8 +1240,8 @@ static int isotp_bind(struct socket *sock, struct sockaddr *uaddr, int len) /* switch to new settings */ so->ifindex = ifindex; - so->rxid = addr->can_addr.tp.rx_id; - so->txid = addr->can_addr.tp.tx_id; + so->rxid = rx_id; + so->txid = tx_id; so->bound = 1; out: From 30ffd5332e06316bd69a654c06aa033872979b7c Mon Sep 17 00:00:00 2001 From: Oliver Hartkopp Date: Wed, 16 Mar 2022 17:42:57 +0100 Subject: [PATCH 2/5] can: isotp: return -EADDRNOTAVAIL when reading from unbound socket When reading from an unbound can-isotp socket the syscall blocked indefinitely. As unbound sockets (without given CAN address information) do not make sense anyway we directly return -EADDRNOTAVAIL on read() analogue to the known behavior from sendmsg(). Fixes: e057dd3fc20f ("can: add ISO 15765-2:2016 transport protocol") Link: https://github.com/linux-can/can-utils/issues/349 Link: https://lore.kernel.org/all/20220316164258.54155-2-socketcan@hartkopp.net Suggested-by: Derek Will Signed-off-by: Oliver Hartkopp Signed-off-by: Marc Kleine-Budde --- net/can/isotp.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/net/can/isotp.c b/net/can/isotp.c index 1662103ce125..6b6c82206c30 100644 --- a/net/can/isotp.c +++ b/net/can/isotp.c @@ -1046,12 +1046,16 @@ static int isotp_recvmsg(struct socket *sock, struct msghdr *msg, size_t size, { struct sock *sk = sock->sk; struct sk_buff *skb; + struct isotp_sock *so = isotp_sk(sk); int err = 0; int noblock; noblock = flags & MSG_DONTWAIT; flags &= ~MSG_DONTWAIT; + if (!so->bound) + return -EADDRNOTAVAIL; + skb = skb_recv_datagram(sk, flags, noblock, &err); if (!skb) return err; From 42bf50a1795a1854d48717b7361dbdbce496b16b Mon Sep 17 00:00:00 2001 From: Oliver Hartkopp Date: Wed, 16 Mar 2022 17:42:58 +0100 Subject: [PATCH 3/5] can: isotp: support MSG_TRUNC flag when reading from socket When providing the MSG_TRUNC flag via recvmsg() syscall the return value provides the real length of the packet or datagram, even when it was longer than the passed buffer. Fixes: e057dd3fc20f ("can: add ISO 15765-2:2016 transport protocol") Link: https://github.com/linux-can/can-utils/issues/347#issuecomment-1065932671 Link: https://lore.kernel.org/all/20220316164258.54155-3-socketcan@hartkopp.net Suggested-by: Derek Will Signed-off-by: Oliver Hartkopp Signed-off-by: Marc Kleine-Budde --- net/can/isotp.c | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/net/can/isotp.c b/net/can/isotp.c index 6b6c82206c30..f6f8ba1f816d 100644 --- a/net/can/isotp.c +++ b/net/can/isotp.c @@ -1047,29 +1047,28 @@ static int isotp_recvmsg(struct socket *sock, struct msghdr *msg, size_t size, struct sock *sk = sock->sk; struct sk_buff *skb; struct isotp_sock *so = isotp_sk(sk); - int err = 0; - int noblock; + int noblock = flags & MSG_DONTWAIT; + int ret = 0; - noblock = flags & MSG_DONTWAIT; - flags &= ~MSG_DONTWAIT; + if (flags & ~(MSG_DONTWAIT | MSG_TRUNC)) + return -EINVAL; if (!so->bound) return -EADDRNOTAVAIL; - skb = skb_recv_datagram(sk, flags, noblock, &err); + flags &= ~MSG_DONTWAIT; + skb = skb_recv_datagram(sk, flags, noblock, &ret); if (!skb) - return err; + return ret; if (size < skb->len) msg->msg_flags |= MSG_TRUNC; else size = skb->len; - err = memcpy_to_msg(msg, skb->data, size); - if (err < 0) { - skb_free_datagram(sk, skb); - return err; - } + ret = memcpy_to_msg(msg, skb->data, size); + if (ret < 0) + goto out_err; sock_recv_timestamp(msg, sk, skb); @@ -1079,9 +1078,13 @@ static int isotp_recvmsg(struct socket *sock, struct msghdr *msg, size_t size, memcpy(msg->msg_name, skb->cb, msg->msg_namelen); } + /* set length of return value */ + ret = (flags & MSG_TRUNC) ? skb->len : size; + +out_err: skb_free_datagram(sk, skb); - return size; + return ret; } static int isotp_release(struct socket *sock) From 7843d3c8e5e6887e79dd11c7b5311d9fec66cb76 Mon Sep 17 00:00:00 2001 From: Amit Kumar Mahapatra Date: Wed, 16 Mar 2022 22:41:05 +0530 Subject: [PATCH 4/5] dt-bindings: can: xilinx_can: Convert Xilinx CAN binding to YAML Convert Xilinx CAN binding documentation to YAML. Link: https://lore.kernel.org/all/20220316171105.17654-1-amit.kumar-mahapatra@xilinx.com Signed-off-by: Amit Kumar Mahapatra Reviewed-by: Krzysztof Kozlowski Signed-off-by: Marc Kleine-Budde --- .../bindings/net/can/xilinx,can.yaml | 161 ++++++++++++++++++ .../bindings/net/can/xilinx_can.txt | 61 ------- 2 files changed, 161 insertions(+), 61 deletions(-) create mode 100644 Documentation/devicetree/bindings/net/can/xilinx,can.yaml delete mode 100644 Documentation/devicetree/bindings/net/can/xilinx_can.txt diff --git a/Documentation/devicetree/bindings/net/can/xilinx,can.yaml b/Documentation/devicetree/bindings/net/can/xilinx,can.yaml new file mode 100644 index 000000000000..65af8183cb9c --- /dev/null +++ b/Documentation/devicetree/bindings/net/can/xilinx,can.yaml @@ -0,0 +1,161 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/net/can/xilinx,can.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: + Xilinx Axi CAN/Zynq CANPS controller + +maintainers: + - Appana Durga Kedareswara rao + +properties: + compatible: + enum: + - xlnx,zynq-can-1.0 + - xlnx,axi-can-1.00.a + - xlnx,canfd-1.0 + - xlnx,canfd-2.0 + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + + clocks: + minItems: 1 + maxItems: 2 + + clock-names: + maxItems: 2 + + power-domains: + maxItems: 1 + + tx-fifo-depth: + $ref: "/schemas/types.yaml#/definitions/uint32" + description: CAN Tx fifo depth (Zynq, Axi CAN). + + rx-fifo-depth: + $ref: "/schemas/types.yaml#/definitions/uint32" + description: CAN Rx fifo depth (Zynq, Axi CAN, CAN FD in sequential Rx mode) + + tx-mailbox-count: + $ref: "/schemas/types.yaml#/definitions/uint32" + description: CAN Tx mailbox buffer count (CAN FD) + +required: + - compatible + - reg + - interrupts + - clocks + - clock-names + +unevaluatedProperties: false + +allOf: + - $ref: can-controller.yaml# + - if: + properties: + compatible: + contains: + enum: + - xlnx,zynq-can-1.0 + + then: + properties: + clock-names: + items: + - const: can_clk + - const: pclk + required: + - tx-fifo-depth + - rx-fifo-depth + + - if: + properties: + compatible: + contains: + enum: + - xlnx,axi-can-1.00.a + + then: + properties: + clock-names: + items: + - const: can_clk + - const: s_axi_aclk + required: + - tx-fifo-depth + - rx-fifo-depth + + - if: + properties: + compatible: + contains: + enum: + - xlnx,canfd-1.0 + - xlnx,canfd-2.0 + + then: + properties: + clock-names: + items: + - const: can_clk + - const: s_axi_aclk + required: + - tx-mailbox-count + - rx-fifo-depth + +examples: + - | + #include + + can@e0008000 { + compatible = "xlnx,zynq-can-1.0"; + reg = <0xe0008000 0x1000>; + clocks = <&clkc 19>, <&clkc 36>; + clock-names = "can_clk", "pclk"; + interrupts = ; + interrupt-parent = <&intc>; + tx-fifo-depth = <0x40>; + rx-fifo-depth = <0x40>; + }; + + - | + can@40000000 { + compatible = "xlnx,axi-can-1.00.a"; + reg = <0x40000000 0x10000>; + clocks = <&clkc 0>, <&clkc 1>; + clock-names = "can_clk", "s_axi_aclk"; + interrupt-parent = <&intc>; + interrupts = ; + tx-fifo-depth = <0x40>; + rx-fifo-depth = <0x40>; + }; + + - | + can@40000000 { + compatible = "xlnx,canfd-1.0"; + reg = <0x40000000 0x2000>; + clocks = <&clkc 0>, <&clkc 1>; + clock-names = "can_clk", "s_axi_aclk"; + interrupt-parent = <&intc>; + interrupts = ; + tx-mailbox-count = <0x20>; + rx-fifo-depth = <0x20>; + }; + + - | + can@ff060000 { + compatible = "xlnx,canfd-2.0"; + reg = <0xff060000 0x6000>; + clocks = <&clkc 0>, <&clkc 1>; + clock-names = "can_clk", "s_axi_aclk"; + interrupt-parent = <&intc>; + interrupts = ; + tx-mailbox-count = <0x20>; + rx-fifo-depth = <0x40>; + }; diff --git a/Documentation/devicetree/bindings/net/can/xilinx_can.txt b/Documentation/devicetree/bindings/net/can/xilinx_can.txt deleted file mode 100644 index 100cc40b8510..000000000000 --- a/Documentation/devicetree/bindings/net/can/xilinx_can.txt +++ /dev/null @@ -1,61 +0,0 @@ -Xilinx Axi CAN/Zynq CANPS controller Device Tree Bindings ---------------------------------------------------------- - -Required properties: -- compatible : Should be: - - "xlnx,zynq-can-1.0" for Zynq CAN controllers - - "xlnx,axi-can-1.00.a" for Axi CAN controllers - - "xlnx,canfd-1.0" for CAN FD controllers - - "xlnx,canfd-2.0" for CAN FD 2.0 controllers -- reg : Physical base address and size of the controller - registers map. -- interrupts : Property with a value describing the interrupt - number. -- clock-names : List of input clock names - - "can_clk", "pclk" (For CANPS), - - "can_clk", "s_axi_aclk" (For AXI CAN and CAN FD). - (See clock bindings for details). -- clocks : Clock phandles (see clock bindings for details). -- tx-fifo-depth : Can Tx fifo depth (Zynq, Axi CAN). -- rx-fifo-depth : Can Rx fifo depth (Zynq, Axi CAN, CAN FD in - sequential Rx mode). -- tx-mailbox-count : Can Tx mailbox buffer count (CAN FD). -- rx-mailbox-count : Can Rx mailbox buffer count (CAN FD in mailbox Rx - mode). - - -Example: - -For Zynq CANPS Dts file: - zynq_can_0: can@e0008000 { - compatible = "xlnx,zynq-can-1.0"; - clocks = <&clkc 19>, <&clkc 36>; - clock-names = "can_clk", "pclk"; - reg = <0xe0008000 0x1000>; - interrupts = <0 28 4>; - interrupt-parent = <&intc>; - tx-fifo-depth = <0x40>; - rx-fifo-depth = <0x40>; - }; -For Axi CAN Dts file: - axi_can_0: axi-can@40000000 { - compatible = "xlnx,axi-can-1.00.a"; - clocks = <&clkc 0>, <&clkc 1>; - clock-names = "can_clk","s_axi_aclk" ; - reg = <0x40000000 0x10000>; - interrupt-parent = <&intc>; - interrupts = <0 59 1>; - tx-fifo-depth = <0x40>; - rx-fifo-depth = <0x40>; - }; -For CAN FD Dts file: - canfd_0: canfd@40000000 { - compatible = "xlnx,canfd-1.0"; - clocks = <&clkc 0>, <&clkc 1>; - clock-names = "can_clk", "s_axi_aclk"; - reg = <0x40000000 0x2000>; - interrupt-parent = <&intc>; - interrupts = <0 59 1>; - tx-mailbox-count = <0x20>; - rx-fifo-depth = <0x20>; - }; From c34983c94166689358372d4af8d5def57752860c Mon Sep 17 00:00:00 2001 From: Julia Lawall Date: Mon, 14 Mar 2022 12:53:51 +0100 Subject: [PATCH 5/5] can: ucan: fix typos in comments Various spelling mistakes in comments. Detected with the help of Coccinelle. Link: https://lore.kernel.org/all/20220314115354.144023-28-Julia.Lawall@inria.fr Signed-off-by: Julia Lawall Acked-by: Marc Kleine-Budde Signed-off-by: Marc Kleine-Budde --- drivers/net/can/usb/ucan.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/can/usb/ucan.c b/drivers/net/can/usb/ucan.c index c7c41d1fd038..5ae0d7c017cc 100644 --- a/drivers/net/can/usb/ucan.c +++ b/drivers/net/can/usb/ucan.c @@ -1392,7 +1392,7 @@ static int ucan_probe(struct usb_interface *intf, * Stage 3 for the final driver initialisation. */ - /* Prepare Memory for control transferes */ + /* Prepare Memory for control transfers */ ctl_msg_buffer = devm_kzalloc(&udev->dev, sizeof(union ucan_ctl_payload), GFP_KERNEL); @@ -1526,7 +1526,7 @@ static int ucan_probe(struct usb_interface *intf, ret = ucan_device_request_in(up, UCAN_DEVICE_GET_FW_STRING, 0, sizeof(union ucan_ctl_payload)); if (ret > 0) { - /* copy string while ensuring zero terminiation */ + /* copy string while ensuring zero termination */ strncpy(firmware_str, up->ctl_msg_buffer->raw, sizeof(union ucan_ctl_payload)); firmware_str[sizeof(union ucan_ctl_payload)] = '\0';