can: isotp: upgrade to latest 6.1 LTS code base

The backport of commit 9c5df2f14ee3 ("can: isotp: isotp_ops: fix poll() to not report false EPOLLOUT events") introduced a new regression where the fix could potentially introduce new side effects.

To reduce the risk of other unmet dependencies and missing fixes and checks the latest 6.1 LTS code base is ported back to the 5.10 LTS tree.

Lukas Magel (1):
can: isotp: isotp_sendmsg(): fix TX state detection and wait behavior

Oliver Hartkopp (6):
can: isotp: set max PDU size to 64 kByte
can: isotp: isotp_bind(): return -EINVAL on incorrect CAN ID formatting
can: isotp: check CAN address family in isotp_bind()
can: isotp: handle wait_event_interruptible() return values
can: isotp: add local echo tx processing and tx without FC
can: isotp: isotp_bind(): do not validate unused address information

Patrick Menschel (3):
can: isotp: change error format from decimal to symbolic error names
can: isotp: add symbolic error message to isotp_module_init()
can: isotp: Add error message if txqueuelen is too small

 include/uapi/linux/can/isotp.h |  25 +-
 net/can/isotp.c                | 434 +++++++++++++++++++++------------
 2 files changed, 293 insertions(+), 166 deletions(-)

2.34.1

From: Patrick Menschel <[email protected]>

commit 46d8657a6b284e32b6b3bf1a6c93ee507fdd3cdb upstream

This patch changes the format string for errors from decimal %d to symbolic error names %pe to achieve more comprehensive log messages.

---
 net/can/isotp.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

index 16ebc187af1c..d34f9ab2eb62 100644
@@ -226,12 +226,12 @@ static int isotp_send_fc(struct sock *sk, int ae, u8 flowstatus)
 
 	ncf->flags = so->ll.tx_flags;
 
 	can_send_ret = can_send(nskb, 1);
 	if (can_send_ret)
-		pr_notice_once("can-isotp: %s: can_send_ret %d\n",
-			       __func__, can_send_ret);
+		pr_notice_once("can-isotp: %s: can_send_ret %pe\n",
+			       __func__, ERR_PTR(can_send_ret));
 
 	dev_put(dev);
 
 	/* reset blocksize counter */
 	so->rx.bs = 0;

@@ -812,12 +812,12 @@ static enum hrtimer_restart isotp_tx_timer_handler(struct hrtimer *hrtimer)
 		skb->dev = dev;
 		can_skb_set_owner(skb, sk);
 
 		can_send_ret = can_send(skb, 1);
 		if (can_send_ret)
-			pr_notice_once("can-isotp: %s: can_send_ret %d\n",
-				       __func__, can_send_ret);
+			pr_notice_once("can-isotp: %s: can_send_ret %pe\n",
+				       __func__, ERR_PTR(can_send_ret));
 
 		if (so->tx.idx >= so->tx.len) {
 			/* we are done */
 			so->tx.state = ISOTP_IDLE;
 			dev_put(dev);

@@ -974,12 +974,12 @@ static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
 	skb->dev = dev;
 	skb->sk = sk;
 	err = can_send(skb, 1);
 	dev_put(dev);
 	if (err) {
-		pr_notice_once("can-isotp: %s: can_send_ret %d\n",
-			       __func__, err);
+		pr_notice_once("can-isotp: %s: can_send_ret %pe\n",
+			       __func__, ERR_PTR(err));
 
 		/* no transmission -> no timeout monitoring */
 		if (hrtimer_sec)
 			hrtimer_cancel(&so->txtimer);
 

2.34.1

This is a note to let you know that I've just added the patch titled

can: isotp: change error format from decimal to symbolic error names

to the 5.10-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
can-isotp-change-error-format-from-decimal-to-symbolic-error-names.patch
and it can be found in the queue-5.10 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

From [email protected] Tue Oct 31 10:30:30 2023
From: Oliver Hartkopp <[email protected]>
Date: Tue, 31 Oct 2023 10:29:09 +0100
Subject: can: isotp: change error format from decimal to symbolic error names
To: [email protected], [email protected], [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], Patrick Menschel <[email protected]>, Oliver Hartkopp <[email protected]>
Message-ID: <[email protected]>

From: Patrick Menschel <[email protected]>

commit 46d8657a6b284e32b6b3bf1a6c93ee507fdd3cdb upstream

This patch changes the format string for errors from decimal %d to symbolic error names %pe to achieve more comprehensive log messages.

---
 net/can/isotp.c |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

@@ -228,8 +228,8 @@ static int isotp_send_fc(struct sock *sk
 
 	can_send_ret = can_send(nskb, 1);
 	if (can_send_ret)
-		pr_notice_once("can-isotp: %s: can_send_ret %d\n",
-			       __func__, can_send_ret);
+		pr_notice_once("can-isotp: %s: can_send_ret %pe\n",
+			       __func__, ERR_PTR(can_send_ret));
 
 	dev_put(dev);
 

@@ -814,8 +814,8 @@ isotp_tx_burst:
 
 		can_send_ret = can_send(skb, 1);
 		if (can_send_ret)
-			pr_notice_once("can-isotp: %s: can_send_ret %d\n",
-				       __func__, can_send_ret);
+			pr_notice_once("can-isotp: %s: can_send_ret %pe\n",
+				       __func__, ERR_PTR(can_send_ret));
 
 		if (so->tx.idx >= so->tx.len) {
 			/* we are done */

@@ -976,8 +976,8 @@ static int isotp_sendmsg(struct socket *
 	err = can_send(skb, 1);
 	dev_put(dev);
 	if (err) {
-		pr_notice_once("can-isotp: %s: can_send_ret %d\n",
-			       __func__, err);
+		pr_notice_once("can-isotp: %s: can_send_ret %pe\n",
+			       __func__, ERR_PTR(err));
 
 		/* no transmission -> no timeout monitoring */
 		if (hrtimer_sec)

Patches currently in stable-queue which might be from [email protected] are

queue-5.10/ext4-avoid-overlapping-preallocations-due-to-overflow.patch queue-5.10/can-isotp-isotp_bind-do-not-validate-unused-address-information.patch queue-5.10/can-isotp-set-max-pdu-size-to-64-kbyte.patch queue-5.10/can-isotp-isotp_bind-return-einval-on-incorrect-can-id-formatting.patch queue-5.10/ext4-fix-bug-in-ext4_mb_new_inode_pa-due-to-overflow.patch queue-5.10/can-isotp-add-symbolic-error-message-to-isotp_module_init.patch queue-5.10/rpmsg-fix-calling-device_lock-on-non-initialized-device.patch queue-5.10/can-isotp-change-error-format-from-decimal-to-symbolic-error-names.patch queue-5.10/can-isotp-isotp_sendmsg-fix-tx-state-detection-and-wait-behavior.patch queue-5.10/rpmsg-constify-local-variable-in-field-store-macro.patch queue-5.10/can-isotp-add-error-message-if-txqueuelen-is-too-small.patch queue-5.10/rpmsg-glink-release-driver_override.patch queue-5.10/driver-platform-add-helper-for-safer-setting-of-driver_override.patch queue-5.10/can-isotp-check-can-address-family-in-isotp_bind.patch queue-5.10/rpmsg-fix-kfree-of-static-memory-on-setting-driver_override.patch queue-5.10/can-isotp-add-local-echo-tx-processing-and-tx-without-fc.patch queue-5.10/can-isotp-handle-wait_event_interruptible-return-values.patch queue-5.10/rpmsg-fix-possible-refcount-leak-in-rpmsg_register_device_override.patch queue-5.10/ext4-add-two-helper-functions-extent_logical_end-and-pa_logical_end.patch

From: Patrick Menschel <[email protected]>

commit 6a5ddae578842652719fb926b22f1d510fe50bee upstream

This patch adds the value of err with format %pe to the already existing error message.

---
 net/can/isotp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index d34f9ab2eb62..009d5216f3ea 100644
@@ -1548,11 +1548,11 @@ static __init int isotp_module_init(void)
 
 	pr_info("can: isotp protocol\n");
 
 	err = can_proto_register(&isotp_can_proto);
 	if (err < 0)
-		pr_err("can: registration of isotp protocol failed\n");
+		pr_err("can: registration of isotp protocol failed %pe\n", ERR_PTR(err));
 	else
 		register_netdevice_notifier(&canisotp_notifier);
 
 	return err;
 }

2.34.1

This is a note to let you know that I've just added the patch titled

can: isotp: add symbolic error message to isotp_module_init()

to the 5.10-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
can-isotp-add-symbolic-error-message-to-isotp_module_init.patch
and it can be found in the queue-5.10 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

From [email protected] Tue Oct 31 10:30:31 2023
From: Oliver Hartkopp <[email protected]>
Date: Tue, 31 Oct 2023 10:29:10 +0100
Subject: can: isotp: add symbolic error message to isotp_module_init()
To: [email protected], [email protected], [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], Patrick Menschel <[email protected]>, Oliver Hartkopp <[email protected]>
Message-ID: <[email protected]>

From: Patrick Menschel <[email protected]>

commit 6a5ddae578842652719fb926b22f1d510fe50bee upstream

This patch adds the value of err with format %pe to the already existing error message.

---
 net/can/isotp.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

@@ -1550,7 +1550,7 @@ static __init int isotp_module_init(void
 
 	err = can_proto_register(&isotp_can_proto);
 	if (err < 0)
-		pr_err("can: registration of isotp protocol failed\n");
+		pr_err("can: registration of isotp protocol failed %pe\n", ERR_PTR(err));
 	else
 		register_netdevice_notifier(&canisotp_notifier);
 

Patches currently in stable-queue which might be from [email protected] are

queue-5.10/ext4-avoid-overlapping-preallocations-due-to-overflow.patch queue-5.10/can-isotp-isotp_bind-do-not-validate-unused-address-information.patch queue-5.10/can-isotp-set-max-pdu-size-to-64-kbyte.patch queue-5.10/can-isotp-isotp_bind-return-einval-on-incorrect-can-id-formatting.patch queue-5.10/ext4-fix-bug-in-ext4_mb_new_inode_pa-due-to-overflow.patch queue-5.10/can-isotp-add-symbolic-error-message-to-isotp_module_init.patch queue-5.10/rpmsg-fix-calling-device_lock-on-non-initialized-device.patch queue-5.10/can-isotp-change-error-format-from-decimal-to-symbolic-error-names.patch queue-5.10/can-isotp-isotp_sendmsg-fix-tx-state-detection-and-wait-behavior.patch queue-5.10/rpmsg-constify-local-variable-in-field-store-macro.patch queue-5.10/can-isotp-add-error-message-if-txqueuelen-is-too-small.patch queue-5.10/rpmsg-glink-release-driver_override.patch queue-5.10/driver-platform-add-helper-for-safer-setting-of-driver_override.patch queue-5.10/can-isotp-check-can-address-family-in-isotp_bind.patch queue-5.10/rpmsg-fix-kfree-of-static-memory-on-setting-driver_override.patch queue-5.10/can-isotp-add-local-echo-tx-processing-and-tx-without-fc.patch queue-5.10/can-isotp-handle-wait_event_interruptible-return-values.patch queue-5.10/rpmsg-fix-possible-refcount-leak-in-rpmsg_register_device_override.patch queue-5.10/ext4-add-two-helper-functions-extent_logical_end-and-pa_logical_end.patch

From: Patrick Menschel <[email protected]>

commit c69d190f7bb9a03cf5237d45a457993730d01605 upstream

This patch adds an additional error message in case that txqueuelen is set too small and advices the user to increase txqueuelen.

This is likely to happen even with small transfers if txqueuelen is at default value 10 frames.

---
 net/can/isotp.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

index 009d5216f3ea..ef72e5344789 100644
@@ -811,14 +811,16 @@ static enum hrtimer_restart isotp_tx_timer_handler(struct hrtimer *hrtimer)
 
 		skb->dev = dev;
 		can_skb_set_owner(skb, sk);
 
 		can_send_ret = can_send(skb, 1);
-		if (can_send_ret)
+		if (can_send_ret) {
 			pr_notice_once("can-isotp: %s: can_send_ret %pe\n",
 				       __func__, ERR_PTR(can_send_ret));
-
+			if (can_send_ret == -ENOBUFS)
+				pr_notice_once("can-isotp: tx queue is full, increasing txqueuelen may prevent this error\n");
+		}
 		if (so->tx.idx >= so->tx.len) {
 			/* we are done */
 			so->tx.state = ISOTP_IDLE;
 			dev_put(dev);
 			wake_up_interruptible(&so->wait);

2.34.1

This is a note to let you know that I've just added the patch titled

can: isotp: Add error message if txqueuelen is too small

to the 5.10-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
can-isotp-add-error-message-if-txqueuelen-is-too-small.patch
and it can be found in the queue-5.10 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

From [email protected] Tue Oct 31 10:30:23 2023
From: Oliver Hartkopp <[email protected]>
Date: Tue, 31 Oct 2023 10:29:11 +0100
Subject: can: isotp: Add error message if txqueuelen is too small
To: [email protected], [email protected], [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], Patrick Menschel <[email protected]>, Oliver Hartkopp <[email protected]>
Message-ID: <[email protected]>

From: Patrick Menschel <[email protected]>

commit c69d190f7bb9a03cf5237d45a457993730d01605 upstream

This patch adds an additional error message in case that txqueuelen is set too small and advices the user to increase txqueuelen.

This is likely to happen even with small transfers if txqueuelen is at default value 10 frames.

---
 net/can/isotp.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

@@ -813,10 +813,12 @@ isotp_tx_burst:
 		can_skb_set_owner(skb, sk);
 
 		can_send_ret = can_send(skb, 1);
-		if (can_send_ret)
+		if (can_send_ret) {
 			pr_notice_once("can-isotp: %s: can_send_ret %pe\n",
 				       __func__, ERR_PTR(can_send_ret));
-
+			if (can_send_ret == -ENOBUFS)
+				pr_notice_once("can-isotp: tx queue is full, increasing txqueuelen may prevent this error\n");
+		}
 		if (so->tx.idx >= so->tx.len) {
 			/* we are done */
 			so->tx.state = ISOTP_IDLE;

Patches currently in stable-queue which might be from [email protected] are

queue-5.10/ext4-avoid-overlapping-preallocations-due-to-overflow.patch queue-5.10/can-isotp-isotp_bind-do-not-validate-unused-address-information.patch queue-5.10/can-isotp-set-max-pdu-size-to-64-kbyte.patch queue-5.10/can-isotp-isotp_bind-return-einval-on-incorrect-can-id-formatting.patch queue-5.10/ext4-fix-bug-in-ext4_mb_new_inode_pa-due-to-overflow.patch queue-5.10/can-isotp-add-symbolic-error-message-to-isotp_module_init.patch queue-5.10/rpmsg-fix-calling-device_lock-on-non-initialized-device.patch queue-5.10/can-isotp-change-error-format-from-decimal-to-symbolic-error-names.patch queue-5.10/can-isotp-isotp_sendmsg-fix-tx-state-detection-and-wait-behavior.patch queue-5.10/rpmsg-constify-local-variable-in-field-store-macro.patch queue-5.10/can-isotp-add-error-message-if-txqueuelen-is-too-small.patch queue-5.10/rpmsg-glink-release-driver_override.patch queue-5.10/driver-platform-add-helper-for-safer-setting-of-driver_override.patch queue-5.10/can-isotp-check-can-address-family-in-isotp_bind.patch queue-5.10/rpmsg-fix-kfree-of-static-memory-on-setting-driver_override.patch queue-5.10/can-isotp-add-local-echo-tx-processing-and-tx-without-fc.patch queue-5.10/can-isotp-handle-wait_event_interruptible-return-values.patch queue-5.10/rpmsg-fix-possible-refcount-leak-in-rpmsg_register_device_override.patch queue-5.10/ext4-add-two-helper-functions-extent_logical_end-and-pa_logical_end.patch

commit 9c0c191d82a1de964ac953a1df8b5744ec670b07 upstream

The reason to extend the max PDU size from 4095 Byte (12 bit length value) to a 32 bit value (up to 4 GByte) was to be able to flash 64 kByte bootloaders with a single ISO-TP PDU. The max PDU size in the Linux kernel implementation was set to 8200 Bytes to be able to test the length information escape sequence.

It turns out that the demand for 64 kByte PDUs is real so the value for MAX_MSG_LENGTH is set to 66000 to be able to potentially add some checksums to the 65.536 Byte block.

---
 net/can/isotp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

index ef72e5344789..fb179a333784 100644
@@ -85,13 +85,13 @@ MODULE_ALIAS("can-proto-6");
 			 (CAN_SFF_MASK | CAN_EFF_FLAG | CAN_RTR_FLAG))
 
 /* ISO 15765-2:2016 supports more than 4095 byte per ISO PDU as the FF_DL can
  * take full 32 bit values (4 Gbyte). We would need some good concept to handle
  * this between user space and kernel space. For now increase the static buffer
- * to something about 8 kbyte to be able to test this new functionality.
+ * to something about 64 kbyte to be able to test this new functionality.
  */
-#define MAX_MSG_LENGTH 8200
+#define MAX_MSG_LENGTH 66000
 
 /* N_PCI type values in bits 7-4 of N_PCI bytes */
 #define N_PCI_SF 0x00	/* single frame */
 #define N_PCI_FF 0x10	/* first frame */
 #define N_PCI_CF 0x20	/* consecutive frame */

2.34.1

This is a note to let you know that I've just added the patch titled

can: isotp: set max PDU size to 64 kByte

to the 5.10-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
can-isotp-set-max-pdu-size-to-64-kbyte.patch
and it can be found in the queue-5.10 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

From [email protected] Tue Oct 31 10:30:21 2023
From: Oliver Hartkopp <[email protected]>
Date: Tue, 31 Oct 2023 10:29:12 +0100
Subject: can: isotp: set max PDU size to 64 kByte
To: [email protected], [email protected], [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], Oliver Hartkopp <[email protected]>
Message-ID: <[email protected]>

From: Oliver Hartkopp <[email protected]>

commit 9c0c191d82a1de964ac953a1df8b5744ec670b07 upstream

The reason to extend the max PDU size from 4095 Byte (12 bit length value) to a 32 bit value (up to 4 GByte) was to be able to flash 64 kByte bootloaders with a single ISO-TP PDU. The max PDU size in the Linux kernel implementation was set to 8200 Bytes to be able to test the length information escape sequence.

It turns out that the demand for 64 kByte PDUs is real so the value for MAX_MSG_LENGTH is set to 66000 to be able to potentially add some checksums to the 65.536 Byte block.

---
 net/can/isotp.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

@@ -87,9 +87,9 @@ MODULE_ALIAS("can-proto-6");
 /* ISO 15765-2:2016 supports more than 4095 byte per ISO PDU as the FF_DL can
  * take full 32 bit values (4 Gbyte). We would need some good concept to handle
  * this between user space and kernel space. For now increase the static buffer
- * to something about 8 kbyte to be able to test this new functionality.
+ * to something about 64 kbyte to be able to test this new functionality.
  */
-#define MAX_MSG_LENGTH 8200
+#define MAX_MSG_LENGTH 66000
 
 /* N_PCI type values in bits 7-4 of N_PCI bytes */
 #define N_PCI_SF 0x00	/* single frame */

Patches currently in stable-queue which might be from [email protected] are

queue-5.10/ext4-avoid-overlapping-preallocations-due-to-overflow.patch queue-5.10/can-isotp-isotp_bind-do-not-validate-unused-address-information.patch queue-5.10/can-isotp-set-max-pdu-size-to-64-kbyte.patch queue-5.10/can-isotp-isotp_bind-return-einval-on-incorrect-can-id-formatting.patch queue-5.10/ext4-fix-bug-in-ext4_mb_new_inode_pa-due-to-overflow.patch queue-5.10/can-isotp-add-symbolic-error-message-to-isotp_module_init.patch queue-5.10/rpmsg-fix-calling-device_lock-on-non-initialized-device.patch queue-5.10/can-isotp-change-error-format-from-decimal-to-symbolic-error-names.patch queue-5.10/can-isotp-isotp_sendmsg-fix-tx-state-detection-and-wait-behavior.patch queue-5.10/rpmsg-constify-local-variable-in-field-store-macro.patch queue-5.10/can-isotp-add-error-message-if-txqueuelen-is-too-small.patch queue-5.10/rpmsg-glink-release-driver_override.patch queue-5.10/driver-platform-add-helper-for-safer-setting-of-driver_override.patch queue-5.10/can-isotp-check-can-address-family-in-isotp_bind.patch queue-5.10/rpmsg-fix-kfree-of-static-memory-on-setting-driver_override.patch queue-5.10/can-isotp-add-local-echo-tx-processing-and-tx-without-fc.patch queue-5.10/can-isotp-handle-wait_event_interruptible-return-values.patch queue-5.10/rpmsg-fix-possible-refcount-leak-in-rpmsg_register_device_override.patch queue-5.10/ext4-add-two-helper-functions-extent_logical_end-and-pa_logical_end.patch

commit 2aa39889c463195a0dfe2aff9fad413139c32a4f upstream

Commit 3ea566422cbd ("can: isotp: sanitize CAN ID checks in isotp_bind()") checks the given CAN ID address information by sanitizing the input values.

This check (silently) removes obsolete bits by masking the given CAN IDs.

Derek Will suggested to give a feedback to the application programmer when the 'sanitizing' was actually needed which means the programmer provided CAN ID content in a wrong format (e.g. SFF CAN IDs with a CAN ID > 0x7FF).

---
 net/can/isotp.c | 5 +++++
 1 file changed, 5 insertions(+)

index fb179a333784..902cb61b6495 100644
@@ -1140,10 +1140,15 @@ static int isotp_bind(struct socket *sock, struct sockaddr *uaddr, int len)
 	if (rx_id & CAN_EFF_FLAG)
 		rx_id &= (CAN_EFF_FLAG | CAN_EFF_MASK);
 	else
 		rx_id &= CAN_SFF_MASK;
 
+	/* give feedback on wrong CAN-ID values */
+	if (tx_id != addr->can_addr.tp.tx_id ||
+	    rx_id != addr->can_addr.tp.rx_id)
+		return -EINVAL;
+
 	if (!addr->can_ifindex)
 		return -ENODEV;
 
 	lock_sock(sk);
 

2.34.1

This is a note to let you know that I've just added the patch titled

can: isotp: isotp_bind(): return -EINVAL on incorrect CAN ID formatting

to the 5.10-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
can-isotp-isotp_bind-return-einval-on-incorrect-can-id-formatting.patch
and it can be found in the queue-5.10 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

From [email protected] Tue Oct 31 10:30:23 2023
From: Oliver Hartkopp <[email protected]>
Date: Tue, 31 Oct 2023 10:29:13 +0100
Subject: can: isotp: isotp_bind(): return -EINVAL on incorrect CAN ID formatting
To: [email protected], [email protected], [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], Oliver Hartkopp <[email protected]>, Derek Will <[email protected]>
Message-ID: <[email protected]>

From: Oliver Hartkopp <[email protected]>

commit 2aa39889c463195a0dfe2aff9fad413139c32a4f upstream

Commit 3ea566422cbd ("can: isotp: sanitize CAN ID checks in isotp_bind()") checks the given CAN ID address information by sanitizing the input values.

This check (silently) removes obsolete bits by masking the given CAN IDs.

Derek Will suggested to give a feedback to the application programmer when the 'sanitizing' was actually needed which means the programmer provided CAN ID content in a wrong format (e.g. SFF CAN IDs with a CAN ID > 0x7FF).

---
 net/can/isotp.c |    5 +++++
 1 file changed, 5 insertions(+)

@@ -1142,6 +1142,11 @@ static int isotp_bind(struct socket *soc
 	else
 		rx_id &= CAN_SFF_MASK;
 
+	/* give feedback on wrong CAN-ID values */
+	if (tx_id != addr->can_addr.tp.tx_id ||
+	    rx_id != addr->can_addr.tp.rx_id)
+		return -EINVAL;
+
 	if (!addr->can_ifindex)
 		return -ENODEV;
 

Patches currently in stable-queue which might be from [email protected] are

queue-5.10/ext4-avoid-overlapping-preallocations-due-to-overflow.patch queue-5.10/can-isotp-isotp_bind-do-not-validate-unused-address-information.patch queue-5.10/can-isotp-set-max-pdu-size-to-64-kbyte.patch queue-5.10/can-isotp-isotp_bind-return-einval-on-incorrect-can-id-formatting.patch queue-5.10/ext4-fix-bug-in-ext4_mb_new_inode_pa-due-to-overflow.patch queue-5.10/can-isotp-add-symbolic-error-message-to-isotp_module_init.patch queue-5.10/rpmsg-fix-calling-device_lock-on-non-initialized-device.patch queue-5.10/can-isotp-change-error-format-from-decimal-to-symbolic-error-names.patch queue-5.10/can-isotp-isotp_sendmsg-fix-tx-state-detection-and-wait-behavior.patch queue-5.10/rpmsg-constify-local-variable-in-field-store-macro.patch queue-5.10/can-isotp-add-error-message-if-txqueuelen-is-too-small.patch queue-5.10/rpmsg-glink-release-driver_override.patch queue-5.10/driver-platform-add-helper-for-safer-setting-of-driver_override.patch queue-5.10/can-isotp-check-can-address-family-in-isotp_bind.patch queue-5.10/rpmsg-fix-kfree-of-static-memory-on-setting-driver_override.patch queue-5.10/can-isotp-add-local-echo-tx-processing-and-tx-without-fc.patch queue-5.10/can-isotp-handle-wait_event_interruptible-return-values.patch queue-5.10/rpmsg-fix-possible-refcount-leak-in-rpmsg_register_device_override.patch queue-5.10/ext4-add-two-helper-functions-extent_logical_end-and-pa_logical_end.patch

commit c6adf659a8ba85913e16a571d5a9bcd17d3d1234 upstream

Add missing check to block non-AF_CAN binds.

Syzbot created some code which matched the right sockaddr struct size but used AF_XDP (0x2C) instead of AF_CAN (0x1D) in the address family field:

bind$xdp(r2, &(0x7f0000000540)={0x2c, 0x0, r4, 0x0, r2}, 0x10)

                                ^^^^

This has no funtional impact but the userspace should be notified about the wrong address family field content.

---
 net/can/isotp.c | 3 +++
 1 file changed, 3 insertions(+)

index 902cb61b6495..87de9a08cc85 100644
@@ -1127,10 +1127,13 @@ static int isotp_bind(struct socket *sock, struct sockaddr *uaddr, int len)
 	int do_rx_reg = 1;
 
 	if (len < ISOTP_MIN_NAMELEN)
 		return -EINVAL;
 
+	if (addr->can_family != AF_CAN)
+		return -EINVAL;
+
 	/* 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

2.34.1

This is a note to let you know that I've just added the patch titled

can: isotp: check CAN address family in isotp_bind()

to the 5.10-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
can-isotp-check-can-address-family-in-isotp_bind.patch
and it can be found in the queue-5.10 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

From [email protected] Tue Oct 31 10:30:27 2023
From: Oliver Hartkopp <[email protected]>
Date: Tue, 31 Oct 2023 10:29:14 +0100
Subject: can: isotp: check CAN address family in isotp_bind()
To: [email protected], [email protected], [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], Oliver Hartkopp <[email protected]>, [email protected]
Message-ID: <[email protected]>

From: Oliver Hartkopp <[email protected]>

commit c6adf659a8ba85913e16a571d5a9bcd17d3d1234 upstream

Add missing check to block non-AF_CAN binds.

Syzbot created some code which matched the right sockaddr struct size but used AF_XDP (0x2C) instead of AF_CAN (0x1D) in the address family field:

bind$xdp(r2, &(0x7f0000000540)={0x2c, 0x0, r4, 0x0, r2}, 0x10)

                                ^^^^

This has no funtional impact but the userspace should be notified about the wrong address family field content.

---
 net/can/isotp.c |    3 +++
 1 file changed, 3 insertions(+)

@@ -1129,6 +1129,9 @@ static int isotp_bind(struct socket *soc
 	if (len < ISOTP_MIN_NAMELEN)
 		return -EINVAL;
 
+	if (addr->can_family != AF_CAN)
+		return -EINVAL;
+
 	/* sanitize tx/rx CAN identifiers */
 	tx_id = addr->can_addr.tp.tx_id;
 	if (tx_id & CAN_EFF_FLAG)

Patches currently in stable-queue which might be from [email protected] are

queue-5.10/ext4-avoid-overlapping-preallocations-due-to-overflow.patch queue-5.10/can-isotp-isotp_bind-do-not-validate-unused-address-information.patch queue-5.10/can-isotp-set-max-pdu-size-to-64-kbyte.patch queue-5.10/can-isotp-isotp_bind-return-einval-on-incorrect-can-id-formatting.patch queue-5.10/ext4-fix-bug-in-ext4_mb_new_inode_pa-due-to-overflow.patch queue-5.10/can-isotp-add-symbolic-error-message-to-isotp_module_init.patch queue-5.10/rpmsg-fix-calling-device_lock-on-non-initialized-device.patch queue-5.10/can-isotp-change-error-format-from-decimal-to-symbolic-error-names.patch queue-5.10/can-isotp-isotp_sendmsg-fix-tx-state-detection-and-wait-behavior.patch queue-5.10/rpmsg-constify-local-variable-in-field-store-macro.patch queue-5.10/can-isotp-add-error-message-if-txqueuelen-is-too-small.patch queue-5.10/rpmsg-glink-release-driver_override.patch queue-5.10/driver-platform-add-helper-for-safer-setting-of-driver_override.patch queue-5.10/can-isotp-check-can-address-family-in-isotp_bind.patch queue-5.10/rpmsg-fix-kfree-of-static-memory-on-setting-driver_override.patch queue-5.10/can-isotp-add-local-echo-tx-processing-and-tx-without-fc.patch queue-5.10/can-isotp-handle-wait_event_interruptible-return-values.patch queue-5.10/rpmsg-fix-possible-refcount-leak-in-rpmsg_register_device_override.patch queue-5.10/ext4-add-two-helper-functions-extent_logical_end-and-pa_logical_end.patch

commit 823b2e42720f96f277940c37ea438b7c5ead51a4 upstream

When wait_event_interruptible() has been interrupted by a signal the tx.state value might not be ISOTP_IDLE. Force the state machines into idle state to inhibit the timer handlers to continue working.

---
 net/can/isotp.c | 4 ++++
 1 file changed, 4 insertions(+)

index 87de9a08cc85..42194b0d176c 100644
@@ -1069,10 +1069,14 @@ static int isotp_release(struct socket *sock)
 	net = sock_net(sk);
 
 	/* wait for complete transmission of current pdu */
 	wait_event_interruptible(so->wait, so->tx.state == ISOTP_IDLE);
 
+	/* force state machines to be idle also when a signal occurred */
+	so->tx.state = ISOTP_IDLE;
+	so->rx.state = ISOTP_IDLE;
+
 	spin_lock(&isotp_notifier_lock);
 	while (isotp_busy_notifier == so) {
 		spin_unlock(&isotp_notifier_lock);
 		schedule_timeout_uninterruptible(1);
 		spin_lock(&isotp_notifier_lock);

2.34.1

This is a note to let you know that I've just added the patch titled

can: isotp: handle wait_event_interruptible() return values

to the 5.10-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
can-isotp-handle-wait_event_interruptible-return-values.patch
and it can be found in the queue-5.10 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

From [email protected] Tue Oct 31 10:30:31 2023
From: Oliver Hartkopp <[email protected]>
Date: Tue, 31 Oct 2023 10:29:15 +0100
Subject: can: isotp: handle wait_event_interruptible() return values
To: [email protected], [email protected], [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], Oliver Hartkopp <[email protected]>
Message-ID: <[email protected]>

From: Oliver Hartkopp <[email protected]>

commit 823b2e42720f96f277940c37ea438b7c5ead51a4 upstream

When wait_event_interruptible() has been interrupted by a signal the tx.state value might not be ISOTP_IDLE. Force the state machines into idle state to inhibit the timer handlers to continue working.

---
 net/can/isotp.c |    4 ++++
 1 file changed, 4 insertions(+)

@@ -1071,6 +1071,10 @@ static int isotp_release(struct socket *
 	/* wait for complete transmission of current pdu */
 	wait_event_interruptible(so->wait, so->tx.state == ISOTP_IDLE);
 
+	/* force state machines to be idle also when a signal occurred */
+	so->tx.state = ISOTP_IDLE;
+	so->rx.state = ISOTP_IDLE;
+
 	spin_lock(&isotp_notifier_lock);
 	while (isotp_busy_notifier == so) {
 		spin_unlock(&isotp_notifier_lock);

Patches currently in stable-queue which might be from [email protected] are

queue-5.10/ext4-avoid-overlapping-preallocations-due-to-overflow.patch queue-5.10/can-isotp-isotp_bind-do-not-validate-unused-address-information.patch queue-5.10/can-isotp-set-max-pdu-size-to-64-kbyte.patch queue-5.10/can-isotp-isotp_bind-return-einval-on-incorrect-can-id-formatting.patch queue-5.10/ext4-fix-bug-in-ext4_mb_new_inode_pa-due-to-overflow.patch queue-5.10/can-isotp-add-symbolic-error-message-to-isotp_module_init.patch queue-5.10/rpmsg-fix-calling-device_lock-on-non-initialized-device.patch queue-5.10/can-isotp-change-error-format-from-decimal-to-symbolic-error-names.patch queue-5.10/can-isotp-isotp_sendmsg-fix-tx-state-detection-and-wait-behavior.patch queue-5.10/rpmsg-constify-local-variable-in-field-store-macro.patch queue-5.10/can-isotp-add-error-message-if-txqueuelen-is-too-small.patch queue-5.10/rpmsg-glink-release-driver_override.patch queue-5.10/driver-platform-add-helper-for-safer-setting-of-driver_override.patch queue-5.10/can-isotp-check-can-address-family-in-isotp_bind.patch queue-5.10/rpmsg-fix-kfree-of-static-memory-on-setting-driver_override.patch queue-5.10/can-isotp-add-local-echo-tx-processing-and-tx-without-fc.patch queue-5.10/can-isotp-handle-wait_event_interruptible-return-values.patch queue-5.10/rpmsg-fix-possible-refcount-leak-in-rpmsg_register_device_override.patch queue-5.10/ext4-add-two-helper-functions-extent_logical_end-and-pa_logical_end.patch

commit 4b7fe92c06901f4563af0e36d25223a5ab343782 upstream commit 9f39d36530e5678d092d53c5c2c60d82b4dcc169 upstream commit 051737439eaee5bdd03d3c2ef5510d54a478fd05 upstream

Due to the existing patch order applied to isotp.c in the stable kernel the original order of depending patches the three original patches 4b7fe92c0690 ("can: isotp: add local echo tx processing for consecutive frames") 9f39d36530e5 ("can: isotp: add support for transmission without flow control") 051737439eae ("can: isotp: fix race between isotp_sendsmg() and isotp_release()") can not be split into different patches that can be applied in working steps to the stable tree.

---
 include/uapi/linux/can/isotp.h |  25 ++-
 net/can/isotp.c                | 398 +++++++++++++++++++++------------
 2 files changed, 267 insertions(+), 156 deletions(-)

index 590f8aea2b6d..439c982f7e81 100644
@@ -122,22 +122,23 @@ struct can_isotp_ll_options {
 				/* by the CAN netdriver configuration	*/
 };
 
 /* flags for isotp behaviour */
 
-#define CAN_ISOTP_LISTEN_MODE	0x001	/* listen only (do not send FC) */
-#define CAN_ISOTP_EXTEND_ADDR	0x002	/* enable extended addressing */
-#define CAN_ISOTP_TX_PADDING	0x004	/* enable CAN frame padding tx path */
-#define CAN_ISOTP_RX_PADDING	0x008	/* enable CAN frame padding rx path */
-#define CAN_ISOTP_CHK_PAD_LEN	0x010	/* check received CAN frame padding */
-#define CAN_ISOTP_CHK_PAD_DATA	0x020	/* check received CAN frame padding */
-#define CAN_ISOTP_HALF_DUPLEX	0x040	/* half duplex error state handling */
-#define CAN_ISOTP_FORCE_TXSTMIN	0x080	/* ignore stmin from received FC */
-#define CAN_ISOTP_FORCE_RXSTMIN	0x100	/* ignore CFs depending on rx stmin */
-#define CAN_ISOTP_RX_EXT_ADDR	0x200	/* different rx extended addressing */
-#define CAN_ISOTP_WAIT_TX_DONE	0x400	/* wait for tx completion */
-#define CAN_ISOTP_SF_BROADCAST	0x800	/* 1-to-N functional addressing */
+#define CAN_ISOTP_LISTEN_MODE	0x0001	/* listen only (do not send FC) */
+#define CAN_ISOTP_EXTEND_ADDR	0x0002	/* enable extended addressing */
+#define CAN_ISOTP_TX_PADDING	0x0004	/* enable CAN frame padding tx path */
+#define CAN_ISOTP_RX_PADDING	0x0008	/* enable CAN frame padding rx path */
+#define CAN_ISOTP_CHK_PAD_LEN	0x0010	/* check received CAN frame padding */
+#define CAN_ISOTP_CHK_PAD_DATA	0x0020	/* check received CAN frame padding */
+#define CAN_ISOTP_HALF_DUPLEX	0x0040	/* half duplex error state handling */
+#define CAN_ISOTP_FORCE_TXSTMIN	0x0080	/* ignore stmin from received FC */
+#define CAN_ISOTP_FORCE_RXSTMIN	0x0100	/* ignore CFs depending on rx stmin */
+#define CAN_ISOTP_RX_EXT_ADDR	0x0200	/* different rx extended addressing */
+#define CAN_ISOTP_WAIT_TX_DONE	0x0400	/* wait for tx completion */
+#define CAN_ISOTP_SF_BROADCAST	0x0800	/* 1-to-N functional addressing */
+#define CAN_ISOTP_CF_BROADCAST	0x1000	/* 1-to-N transmission w/o FC */
 
 /* protocol machine default values */
 
 #define CAN_ISOTP_DEFAULT_FLAGS		0
 #define CAN_ISOTP_DEFAULT_EXT_ADDRESS	0x00

2.34.1

This is a note to let you know that I've just added the patch titled

can: isotp: add local echo tx processing and tx without FC

to the 5.10-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
can-isotp-add-local-echo-tx-processing-and-tx-without-fc.patch
and it can be found in the queue-5.10 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

From [email protected] Tue Oct 31 10:30:31 2023
From: Oliver Hartkopp <[email protected]>
Date: Tue, 31 Oct 2023 10:29:16 +0100
Subject: can: isotp: add local echo tx processing and tx without FC
To: [email protected], [email protected], [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], Oliver Hartkopp <[email protected]>
Message-ID: <[email protected]>

From: Oliver Hartkopp <[email protected]>

commit 4b7fe92c06901f4563af0e36d25223a5ab343782 upstream commit 9f39d36530e5678d092d53c5c2c60d82b4dcc169 upstream commit 051737439eaee5bdd03d3c2ef5510d54a478fd05 upstream

Due to the existing patch order applied to isotp.c in the stable kernel the original order of depending patches the three original patches 4b7fe92c0690 ("can: isotp: add local echo tx processing for consecutive frames") 9f39d36530e5 ("can: isotp: add support for transmission without flow control") 051737439eae ("can: isotp: fix race between isotp_sendsmg() and isotp_release()") can not be split into different patches that can be applied in working steps to the stable tree.

---
 include/uapi/linux/can/isotp.h |   25 +-
 net/can/isotp.c                |  406 ++++++++++++++++++++++++++---------------
 2 files changed, 271 insertions(+), 160 deletions(-)

@@ -124,18 +124,19 @@ struct can_isotp_ll_options {
 
 /* flags for isotp behaviour */
 
-#define CAN_ISOTP_LISTEN_MODE	0x001	/* listen only (do not send FC) */
-#define CAN_ISOTP_EXTEND_ADDR	0x002	/* enable extended addressing */
-#define CAN_ISOTP_TX_PADDING	0x004	/* enable CAN frame padding tx path */
-#define CAN_ISOTP_RX_PADDING	0x008	/* enable CAN frame padding rx path */
-#define CAN_ISOTP_CHK_PAD_LEN	0x010	/* check received CAN frame padding */
-#define CAN_ISOTP_CHK_PAD_DATA	0x020	/* check received CAN frame padding */
-#define CAN_ISOTP_HALF_DUPLEX	0x040	/* half duplex error state handling */
-#define CAN_ISOTP_FORCE_TXSTMIN	0x080	/* ignore stmin from received FC */
-#define CAN_ISOTP_FORCE_RXSTMIN	0x100	/* ignore CFs depending on rx stmin */
-#define CAN_ISOTP_RX_EXT_ADDR	0x200	/* different rx extended addressing */
-#define CAN_ISOTP_WAIT_TX_DONE	0x400	/* wait for tx completion */
-#define CAN_ISOTP_SF_BROADCAST	0x800	/* 1-to-N functional addressing */
+#define CAN_ISOTP_LISTEN_MODE	0x0001	/* listen only (do not send FC) */
+#define CAN_ISOTP_EXTEND_ADDR	0x0002	/* enable extended addressing */
+#define CAN_ISOTP_TX_PADDING	0x0004	/* enable CAN frame padding tx path */
+#define CAN_ISOTP_RX_PADDING	0x0008	/* enable CAN frame padding rx path */
+#define CAN_ISOTP_CHK_PAD_LEN	0x0010	/* check received CAN frame padding */
+#define CAN_ISOTP_CHK_PAD_DATA	0x0020	/* check received CAN frame padding */
+#define CAN_ISOTP_HALF_DUPLEX	0x0040	/* half duplex error state handling */
+#define CAN_ISOTP_FORCE_TXSTMIN	0x0080	/* ignore stmin from received FC */
+#define CAN_ISOTP_FORCE_RXSTMIN	0x0100	/* ignore CFs depending on rx stmin */
+#define CAN_ISOTP_RX_EXT_ADDR	0x0200	/* different rx extended addressing */
+#define CAN_ISOTP_WAIT_TX_DONE	0x0400	/* wait for tx completion */
+#define CAN_ISOTP_SF_BROADCAST	0x0800	/* 1-to-N functional addressing */
+#define CAN_ISOTP_CF_BROADCAST	0x1000	/* 1-to-N transmission w/o FC */
 
 /* protocol machine default values */
 

Patches currently in stable-queue which might be from [email protected] are

queue-5.10/ext4-avoid-overlapping-preallocations-due-to-overflow.patch queue-5.10/can-isotp-isotp_bind-do-not-validate-unused-address-information.patch queue-5.10/can-isotp-set-max-pdu-size-to-64-kbyte.patch queue-5.10/can-isotp-isotp_bind-return-einval-on-incorrect-can-id-formatting.patch queue-5.10/ext4-fix-bug-in-ext4_mb_new_inode_pa-due-to-overflow.patch queue-5.10/can-isotp-add-symbolic-error-message-to-isotp_module_init.patch queue-5.10/rpmsg-fix-calling-device_lock-on-non-initialized-device.patch queue-5.10/can-isotp-change-error-format-from-decimal-to-symbolic-error-names.patch queue-5.10/can-isotp-isotp_sendmsg-fix-tx-state-detection-and-wait-behavior.patch queue-5.10/rpmsg-constify-local-variable-in-field-store-macro.patch queue-5.10/can-isotp-add-error-message-if-txqueuelen-is-too-small.patch queue-5.10/rpmsg-glink-release-driver_override.patch queue-5.10/driver-platform-add-helper-for-safer-setting-of-driver_override.patch queue-5.10/can-isotp-check-can-address-family-in-isotp_bind.patch queue-5.10/rpmsg-fix-kfree-of-static-memory-on-setting-driver_override.patch queue-5.10/can-isotp-add-local-echo-tx-processing-and-tx-without-fc.patch queue-5.10/can-isotp-handle-wait_event_interruptible-return-values.patch queue-5.10/rpmsg-fix-possible-refcount-leak-in-rpmsg_register_device_override.patch queue-5.10/ext4-add-two-helper-functions-extent_logical_end-and-pa_logical_end.patch

commit b76b163f46b661499921a0049982764a6659bfe7 upstream

With commit 2aa39889c463 ("can: isotp: isotp_bind(): return -EINVAL on incorrect CAN ID formatting") the bind() syscall returns -EINVAL when the given CAN ID needed to be sanitized. But in the case of an unconfirmed broadcast mode the rx CAN ID is not needed and may be uninitialized from the caller - which is ok.

This patch makes sure the result of an inproper CAN ID format is only provided when the address information is needed.

---
 net/can/isotp.c | 29 +++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)

index f362b50484fc..08dfa34d68d5 100644
@@ -1214,38 +1214,43 @@ static int isotp_bind(struct socket *sock, struct sockaddr *uaddr, int len)
 	struct sock *sk = sock->sk;
 	struct isotp_sock *so = isotp_sk(sk);
 	struct net *net = sock_net(sk);
 	int ifindex;
 	struct net_device *dev;
-	canid_t tx_id, rx_id;
+	canid_t tx_id = addr->can_addr.tp.tx_id;
+	canid_t rx_id = addr->can_addr.tp.rx_id;
 	int err = 0;
 	int notify_enetdown = 0;
 
 	if (len < ISOTP_MIN_NAMELEN)
 		return -EINVAL;
 
 	if (addr->can_family != AF_CAN)
 		return -EINVAL;
 
-	/* sanitize tx/rx CAN identifiers */
-	tx_id = addr->can_addr.tp.tx_id;
+	/* sanitize tx CAN identifier */
 	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;
-
-	/* give feedback on wrong CAN-ID values */
-	if (tx_id != addr->can_addr.tp.tx_id ||
-	    rx_id != addr->can_addr.tp.rx_id)
+	/* give feedback on wrong CAN-ID value */
+	if (tx_id != addr->can_addr.tp.tx_id)
 		return -EINVAL;
 
+	/* sanitize rx CAN identifier (if needed) */
+	if (isotp_register_rxid(so)) {
+		if (rx_id & CAN_EFF_FLAG)
+			rx_id &= (CAN_EFF_FLAG | CAN_EFF_MASK);
+		else
+			rx_id &= CAN_SFF_MASK;
+
+		/* give feedback on wrong CAN-ID value */
+		if (rx_id != addr->can_addr.tp.rx_id)
+			return -EINVAL;
+	}
+
 	if (!addr->can_ifindex)
 		return -ENODEV;
 
 	lock_sock(sk);
 

2.34.1

This is a note to let you know that I've just added the patch titled

can: isotp: isotp_bind(): do not validate unused address information

to the 5.10-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
can-isotp-isotp_bind-do-not-validate-unused-address-information.patch
and it can be found in the queue-5.10 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

From [email protected] Tue Oct 31 10:30:27 2023
From: Oliver Hartkopp <[email protected]>
Date: Tue, 31 Oct 2023 10:29:17 +0100
Subject: can: isotp: isotp_bind(): do not validate unused address information
To: [email protected], [email protected], [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], Oliver Hartkopp <[email protected]>
Message-ID: <[email protected]>

From: Oliver Hartkopp <[email protected]>

commit b76b163f46b661499921a0049982764a6659bfe7 upstream

With commit 2aa39889c463 ("can: isotp: isotp_bind(): return -EINVAL on incorrect CAN ID formatting") the bind() syscall returns -EINVAL when the given CAN ID needed to be sanitized. But in the case of an unconfirmed broadcast mode the rx CAN ID is not needed and may be uninitialized from the caller - which is ok.

This patch makes sure the result of an inproper CAN ID format is only provided when the address information is needed.

---
 net/can/isotp.c |   29 +++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)

@@ -1216,7 +1216,8 @@ static int isotp_bind(struct socket *soc
 	struct net *net = sock_net(sk);
 	int ifindex;
 	struct net_device *dev;
-	canid_t tx_id, rx_id;
+	canid_t tx_id = addr->can_addr.tp.tx_id;
+	canid_t rx_id = addr->can_addr.tp.rx_id;
 	int err = 0;
 	int notify_enetdown = 0;
 

@@ -1226,24 +1227,28 @@ static int isotp_bind(struct socket *soc
 	if (addr->can_family != AF_CAN)
 		return -EINVAL;
 
-	/* sanitize tx/rx CAN identifiers */
-	tx_id = addr->can_addr.tp.tx_id;
+	/* sanitize tx CAN identifier */
 	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;
-
-	/* give feedback on wrong CAN-ID values */
-	if (tx_id != addr->can_addr.tp.tx_id ||
-	    rx_id != addr->can_addr.tp.rx_id)
+	/* give feedback on wrong CAN-ID value */
+	if (tx_id != addr->can_addr.tp.tx_id)
 		return -EINVAL;
 
+	/* sanitize rx CAN identifier (if needed) */
+	if (isotp_register_rxid(so)) {
+		if (rx_id & CAN_EFF_FLAG)
+			rx_id &= (CAN_EFF_FLAG | CAN_EFF_MASK);
+		else
+			rx_id &= CAN_SFF_MASK;
+
+		/* give feedback on wrong CAN-ID value */
+		if (rx_id != addr->can_addr.tp.rx_id)
+			return -EINVAL;
+	}
+
 	if (!addr->can_ifindex)
 		return -ENODEV;
 

Patches currently in stable-queue which might be from [email protected] are

queue-5.10/ext4-avoid-overlapping-preallocations-due-to-overflow.patch queue-5.10/can-isotp-isotp_bind-do-not-validate-unused-address-information.patch queue-5.10/can-isotp-set-max-pdu-size-to-64-kbyte.patch queue-5.10/can-isotp-isotp_bind-return-einval-on-incorrect-can-id-formatting.patch queue-5.10/ext4-fix-bug-in-ext4_mb_new_inode_pa-due-to-overflow.patch queue-5.10/can-isotp-add-symbolic-error-message-to-isotp_module_init.patch queue-5.10/rpmsg-fix-calling-device_lock-on-non-initialized-device.patch queue-5.10/can-isotp-change-error-format-from-decimal-to-symbolic-error-names.patch queue-5.10/can-isotp-isotp_sendmsg-fix-tx-state-detection-and-wait-behavior.patch queue-5.10/rpmsg-constify-local-variable-in-field-store-macro.patch queue-5.10/can-isotp-add-error-message-if-txqueuelen-is-too-small.patch queue-5.10/rpmsg-glink-release-driver_override.patch queue-5.10/driver-platform-add-helper-for-safer-setting-of-driver_override.patch queue-5.10/can-isotp-check-can-address-family-in-isotp_bind.patch queue-5.10/rpmsg-fix-kfree-of-static-memory-on-setting-driver_override.patch queue-5.10/can-isotp-add-local-echo-tx-processing-and-tx-without-fc.patch queue-5.10/can-isotp-handle-wait_event_interruptible-return-values.patch queue-5.10/rpmsg-fix-possible-refcount-leak-in-rpmsg_register_device_override.patch queue-5.10/ext4-add-two-helper-functions-extent_logical_end-and-pa_logical_end.patch

From: Lukas Magel <[email protected]>

[ Upstream commit d9c2ba65e651467de739324d978b04ed8729f483 ]

With patch [1], isotp_poll was updated to also queue the poller in the so->wait queue, which is used for send state changes. Since the queue now also contains polling tasks that are not interested in sending, the queue fill state can no longer be used as an indication of send readiness. As a consequence, nonblocking writes can lead to a race and lock-up of the socket if there is a second task polling the socket in parallel.

With this patch, isotp_sendmsg does not consult wq_has_sleepers but instead tries to atomically set so->tx.state and waits on so->wait if it is unable to do so. This behavior is in alignment with isotp_poll, which also checks so->tx.state to determine send readiness.

V2:
- Revert direct exit to goto err_event_drop

[1] https://lore.kernel.org/all/[email protected]

---
 net/can/isotp.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

index 08dfa34d68d5..c646fef8f3ba 100644
@@ -923,25 +923,22 @@ static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
 	int err;
 
 	if (!so->bound || so->tx.state == ISOTP_SHUTDOWN)
 		return -EADDRNOTAVAIL;
 
-wait_free_buffer:
-	/* we do not support multiple buffers - for now */
-	if (wq_has_sleeper(&so->wait) && (msg->msg_flags & MSG_DONTWAIT))
-		return -EAGAIN;
+	while (cmpxchg(&so->tx.state, ISOTP_IDLE, ISOTP_SENDING) != ISOTP_IDLE) {
+		/* we do not support multiple buffers - for now */
+		if (msg->msg_flags & MSG_DONTWAIT)
+			return -EAGAIN;
 
-	/* wait for complete transmission of current pdu */
-	err = wait_event_interruptible(so->wait, so->tx.state == ISOTP_IDLE);
-	if (err)
-		goto err_event_drop;
-
-	if (cmpxchg(&so->tx.state, ISOTP_IDLE, ISOTP_SENDING) != ISOTP_IDLE) {
 		if (so->tx.state == ISOTP_SHUTDOWN)
 			return -EADDRNOTAVAIL;
 
-		goto wait_free_buffer;
+		/* wait for complete transmission of current pdu */
+		err = wait_event_interruptible(so->wait, so->tx.state == ISOTP_IDLE);
+		if (err)
+			goto err_event_drop;
 	}
 
 	if (!size || size > MAX_MSG_LENGTH) {
 		err = -EINVAL;
 		goto err_out_drop;

2.34.1

This is a note to let you know that I've just added the patch titled

can: isotp: isotp_sendmsg(): fix TX state detection and wait behavior

to the 5.10-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
can-isotp-isotp_sendmsg-fix-tx-state-detection-and-wait-behavior.patch
and it can be found in the queue-5.10 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

From [email protected] Tue Oct 31 10:30:27 2023
From: Oliver Hartkopp <[email protected]>
Date: Tue, 31 Oct 2023 10:29:18 +0100
Subject: can: isotp: isotp_sendmsg(): fix TX state detection and wait behavior
To: [email protected], [email protected], [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], Oliver Hartkopp <[email protected]>
Message-ID: <[email protected]>

From: Lukas Magel <[email protected]>

[ Upstream commit d9c2ba65e651467de739324d978b04ed8729f483 ]

With patch [1], isotp_poll was updated to also queue the poller in the so->wait queue, which is used for send state changes. Since the queue now also contains polling tasks that are not interested in sending, the queue fill state can no longer be used as an indication of send readiness. As a consequence, nonblocking writes can lead to a race and lock-up of the socket if there is a second task polling the socket in parallel.

With this patch, isotp_sendmsg does not consult wq_has_sleepers but instead tries to atomically set so->tx.state and waits on so->wait if it is unable to do so. This behavior is in alignment with isotp_poll, which also checks so->tx.state to determine send readiness.

V2:
- Revert direct exit to goto err_event_drop

[1] https://lore.kernel.org/all/[email protected]

---
 net/can/isotp.c |   19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

@@ -925,21 +925,18 @@ static int isotp_sendmsg(struct socket *
 	if (!so->bound || so->tx.state == ISOTP_SHUTDOWN)
 		return -EADDRNOTAVAIL;
 
-wait_free_buffer:
-	/* we do not support multiple buffers - for now */
-	if (wq_has_sleeper(&so->wait) && (msg->msg_flags & MSG_DONTWAIT))
-		return -EAGAIN;
+	while (cmpxchg(&so->tx.state, ISOTP_IDLE, ISOTP_SENDING) != ISOTP_IDLE) {
+		/* we do not support multiple buffers - for now */
+		if (msg->msg_flags & MSG_DONTWAIT)
+			return -EAGAIN;
 
-	/* wait for complete transmission of current pdu */
-	err = wait_event_interruptible(so->wait, so->tx.state == ISOTP_IDLE);
-	if (err)
-		goto err_event_drop;
-
-	if (cmpxchg(&so->tx.state, ISOTP_IDLE, ISOTP_SENDING) != ISOTP_IDLE) {
 		if (so->tx.state == ISOTP_SHUTDOWN)
 			return -EADDRNOTAVAIL;
 
-		goto wait_free_buffer;
+		/* wait for complete transmission of current pdu */
+		err = wait_event_interruptible(so->wait, so->tx.state == ISOTP_IDLE);
+		if (err)
+			goto err_event_drop;
 	}
 
 	if (!size || size > MAX_MSG_LENGTH) {

Patches currently in stable-queue which might be from [email protected] are

queue-5.10/ext4-avoid-overlapping-preallocations-due-to-overflow.patch queue-5.10/can-isotp-isotp_bind-do-not-validate-unused-address-information.patch queue-5.10/can-isotp-set-max-pdu-size-to-64-kbyte.patch queue-5.10/can-isotp-isotp_bind-return-einval-on-incorrect-can-id-formatting.patch queue-5.10/ext4-fix-bug-in-ext4_mb_new_inode_pa-due-to-overflow.patch queue-5.10/can-isotp-add-symbolic-error-message-to-isotp_module_init.patch queue-5.10/rpmsg-fix-calling-device_lock-on-non-initialized-device.patch queue-5.10/can-isotp-change-error-format-from-decimal-to-symbolic-error-names.patch queue-5.10/can-isotp-isotp_sendmsg-fix-tx-state-detection-and-wait-behavior.patch queue-5.10/rpmsg-constify-local-variable-in-field-store-macro.patch queue-5.10/can-isotp-add-error-message-if-txqueuelen-is-too-small.patch queue-5.10/rpmsg-glink-release-driver_override.patch queue-5.10/driver-platform-add-helper-for-safer-setting-of-driver_override.patch queue-5.10/can-isotp-check-can-address-family-in-isotp_bind.patch queue-5.10/rpmsg-fix-kfree-of-static-memory-on-setting-driver_override.patch queue-5.10/can-isotp-add-local-echo-tx-processing-and-tx-without-fc.patch queue-5.10/can-isotp-handle-wait_event_interruptible-return-values.patch queue-5.10/rpmsg-fix-possible-refcount-leak-in-rpmsg_register_device_override.patch queue-5.10/ext4-add-two-helper-functions-extent_logical_end-and-pa_logical_end.patch

Both series now queued up, thanks.

greg k-h