Commit 43a08c3b authored by Ziyang Xuan's avatar Ziyang Xuan Committed by Marc Kleine-Budde

can: isotp: isotp_sendmsg(): fix TX buffer concurrent access in isotp_sendmsg()

When isotp_sendmsg() concurrent, tx.state of all TX processes can be
ISOTP_IDLE. The conditions so->tx.state != ISOTP_IDLE and
wq_has_sleeper(&so->wait) can not protect TX buffer from being
accessed by multiple TX processes.

We can use cmpxchg() to try to modify tx.state to ISOTP_SENDING firstly.
If the modification of the previous process succeed, the later process
must wait tx.state to ISOTP_IDLE firstly. Thus, we can ensure TX buffer
is accessed by only one process at the same time. And we should also
restore the original tx.state at the subsequent error processes.

Fixes: e057dd3f ("can: add ISO 15765-2:2016 transport protocol")
Link: https://lore.kernel.org/all/c2517874fbdf4188585cf9ddf67a8fa74d5dbde5.1633764159.git.william.xuanziyang@huawei.com
Cc: stable@vger.kernel.org
Signed-off-by: default avatarZiyang Xuan <william.xuanziyang@huawei.com>
Acked-by: default avatarOliver Hartkopp <socketcan@hartkopp.net>
Signed-off-by: default avatarMarc Kleine-Budde <mkl@pengutronix.de>
parent 9acf6362
...@@ -121,7 +121,7 @@ enum { ...@@ -121,7 +121,7 @@ enum {
struct tpcon { struct tpcon {
int idx; int idx;
int len; int len;
u8 state; u32 state;
u8 bs; u8 bs;
u8 sn; u8 sn;
u8 ll_dl; u8 ll_dl;
...@@ -848,6 +848,7 @@ static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size) ...@@ -848,6 +848,7 @@ static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
{ {
struct sock *sk = sock->sk; struct sock *sk = sock->sk;
struct isotp_sock *so = isotp_sk(sk); struct isotp_sock *so = isotp_sk(sk);
u32 old_state = so->tx.state;
struct sk_buff *skb; struct sk_buff *skb;
struct net_device *dev; struct net_device *dev;
struct canfd_frame *cf; struct canfd_frame *cf;
...@@ -860,47 +861,55 @@ static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size) ...@@ -860,47 +861,55 @@ static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
return -EADDRNOTAVAIL; return -EADDRNOTAVAIL;
/* we do not support multiple buffers - for now */ /* we do not support multiple buffers - for now */
if (so->tx.state != ISOTP_IDLE || wq_has_sleeper(&so->wait)) { if (cmpxchg(&so->tx.state, ISOTP_IDLE, ISOTP_SENDING) != ISOTP_IDLE ||
if (msg->msg_flags & MSG_DONTWAIT) wq_has_sleeper(&so->wait)) {
return -EAGAIN; if (msg->msg_flags & MSG_DONTWAIT) {
err = -EAGAIN;
goto err_out;
}
/* wait for complete transmission of current pdu */ /* wait for complete transmission of current pdu */
err = wait_event_interruptible(so->wait, so->tx.state == ISOTP_IDLE); err = wait_event_interruptible(so->wait, so->tx.state == ISOTP_IDLE);
if (err) if (err)
return err; goto err_out;
} }
if (!size || size > MAX_MSG_LENGTH) if (!size || size > MAX_MSG_LENGTH) {
return -EINVAL; err = -EINVAL;
goto err_out;
}
/* take care of a potential SF_DL ESC offset for TX_DL > 8 */ /* take care of a potential SF_DL ESC offset for TX_DL > 8 */
off = (so->tx.ll_dl > CAN_MAX_DLEN) ? 1 : 0; off = (so->tx.ll_dl > CAN_MAX_DLEN) ? 1 : 0;
/* does the given data fit into a single frame for SF_BROADCAST? */ /* does the given data fit into a single frame for SF_BROADCAST? */
if ((so->opt.flags & CAN_ISOTP_SF_BROADCAST) && if ((so->opt.flags & CAN_ISOTP_SF_BROADCAST) &&
(size > so->tx.ll_dl - SF_PCI_SZ4 - ae - off)) (size > so->tx.ll_dl - SF_PCI_SZ4 - ae - off)) {
return -EINVAL; err = -EINVAL;
goto err_out;
}
err = memcpy_from_msg(so->tx.buf, msg, size); err = memcpy_from_msg(so->tx.buf, msg, size);
if (err < 0) if (err < 0)
return err; goto err_out;
dev = dev_get_by_index(sock_net(sk), so->ifindex); dev = dev_get_by_index(sock_net(sk), so->ifindex);
if (!dev) if (!dev) {
return -ENXIO; err = -ENXIO;
goto err_out;
}
skb = sock_alloc_send_skb(sk, so->ll.mtu + sizeof(struct can_skb_priv), skb = sock_alloc_send_skb(sk, so->ll.mtu + sizeof(struct can_skb_priv),
msg->msg_flags & MSG_DONTWAIT, &err); msg->msg_flags & MSG_DONTWAIT, &err);
if (!skb) { if (!skb) {
dev_put(dev); dev_put(dev);
return err; goto err_out;
} }
can_skb_reserve(skb); can_skb_reserve(skb);
can_skb_prv(skb)->ifindex = dev->ifindex; can_skb_prv(skb)->ifindex = dev->ifindex;
can_skb_prv(skb)->skbcnt = 0; can_skb_prv(skb)->skbcnt = 0;
so->tx.state = ISOTP_SENDING;
so->tx.len = size; so->tx.len = size;
so->tx.idx = 0; so->tx.idx = 0;
...@@ -956,7 +965,7 @@ static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size) ...@@ -956,7 +965,7 @@ static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
if (err) { if (err) {
pr_notice_once("can-isotp: %s: can_send_ret %pe\n", pr_notice_once("can-isotp: %s: can_send_ret %pe\n",
__func__, ERR_PTR(err)); __func__, ERR_PTR(err));
return err; goto err_out;
} }
if (wait_tx_done) { if (wait_tx_done) {
...@@ -965,6 +974,13 @@ static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size) ...@@ -965,6 +974,13 @@ static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
} }
return size; return size;
err_out:
so->tx.state = old_state;
if (so->tx.state == ISOTP_IDLE)
wake_up_interruptible(&so->wait);
return err;
} }
static int isotp_recvmsg(struct socket *sock, struct msghdr *msg, size_t size, static int isotp_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment