Commit b97ee72a authored by Jakub Kicinski's avatar Jakub Kicinski

Merge tag 'linux-can-fixes-for-6.3-20230405' of...

Merge tag 'linux-can-fixes-for-6.3-20230405' of git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can

Marc Kleine-Budde says:

====================
pull-request: can 2023-04-05

The first patch is by Oleksij Rempel and fixes a out-of-bounds memory
access in the j1939 protocol.

The remaining 3 patches target the ISOTP protocol. Oliver Hartkopp
fixes the ISOTP protocol to pass information about dropped PDUs to the
user space via control messages. Michal Sojka's patch fixes poll() to
not forward false EPOLLOUT events. And Oliver Hartkopp fixes a race
condition between isotp_sendsmg() and isotp_release().

* tag 'linux-can-fixes-for-6.3-20230405' of git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can:
  can: isotp: fix race between isotp_sendsmg() and isotp_release()
  can: isotp: isotp_ops: fix poll() to not report false EPOLLOUT events
  can: isotp: isotp_recvmsg(): use sock_recv_cmsgs() to get SOCK_RXQ_OVFL infos
  can: j1939: j1939_tp_tx_dat_new(): fix out-of-bounds memory access
====================

Link: https://lore.kernel.org/r/20230405092444.1802340-1-mkl@pengutronix.deSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parents 4181b39a 05173743
...@@ -119,7 +119,8 @@ enum { ...@@ -119,7 +119,8 @@ enum {
ISOTP_WAIT_FIRST_FC, ISOTP_WAIT_FIRST_FC,
ISOTP_WAIT_FC, ISOTP_WAIT_FC,
ISOTP_WAIT_DATA, ISOTP_WAIT_DATA,
ISOTP_SENDING ISOTP_SENDING,
ISOTP_SHUTDOWN,
}; };
struct tpcon { struct tpcon {
...@@ -880,8 +881,8 @@ static enum hrtimer_restart isotp_tx_timer_handler(struct hrtimer *hrtimer) ...@@ -880,8 +881,8 @@ static enum hrtimer_restart isotp_tx_timer_handler(struct hrtimer *hrtimer)
txtimer); txtimer);
struct sock *sk = &so->sk; struct sock *sk = &so->sk;
/* don't handle timeouts in IDLE state */ /* don't handle timeouts in IDLE or SHUTDOWN state */
if (so->tx.state == ISOTP_IDLE) if (so->tx.state == ISOTP_IDLE || so->tx.state == ISOTP_SHUTDOWN)
return HRTIMER_NORESTART; return HRTIMER_NORESTART;
/* we did not get any flow control or echo frame in time */ /* we did not get any flow control or echo frame in time */
...@@ -918,7 +919,6 @@ static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size) ...@@ -918,7 +919,6 @@ 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;
...@@ -928,23 +928,24 @@ static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size) ...@@ -928,23 +928,24 @@ static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
int off; int off;
int err; int err;
if (!so->bound) if (!so->bound || so->tx.state == ISOTP_SHUTDOWN)
return -EADDRNOTAVAIL; return -EADDRNOTAVAIL;
wait_free_buffer:
/* we do not support multiple buffers - for now */ /* we do not support multiple buffers - for now */
if (cmpxchg(&so->tx.state, ISOTP_IDLE, ISOTP_SENDING) != ISOTP_IDLE || if (wq_has_sleeper(&so->wait) && (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)
goto err_out; goto err_event_drop;
so->tx.state = ISOTP_SENDING; if (cmpxchg(&so->tx.state, ISOTP_IDLE, ISOTP_SENDING) != ISOTP_IDLE) {
if (so->tx.state == ISOTP_SHUTDOWN)
return -EADDRNOTAVAIL;
goto wait_free_buffer;
} }
if (!size || size > MAX_MSG_LENGTH) { if (!size || size > MAX_MSG_LENGTH) {
...@@ -1074,7 +1075,9 @@ static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size) ...@@ -1074,7 +1075,9 @@ static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
if (wait_tx_done) { if (wait_tx_done) {
/* wait for complete transmission of current pdu */ /* wait for complete transmission of current pdu */
wait_event_interruptible(so->wait, so->tx.state == ISOTP_IDLE); err = wait_event_interruptible(so->wait, so->tx.state == ISOTP_IDLE);
if (err)
goto err_event_drop;
if (sk->sk_err) if (sk->sk_err)
return -sk->sk_err; return -sk->sk_err;
...@@ -1082,13 +1085,15 @@ static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size) ...@@ -1082,13 +1085,15 @@ static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
return size; return size;
err_event_drop:
/* got signal: force tx state machine to be idle */
so->tx.state = ISOTP_IDLE;
hrtimer_cancel(&so->txfrtimer);
hrtimer_cancel(&so->txtimer);
err_out_drop: err_out_drop:
/* drop this PDU and unlock a potential wait queue */ /* drop this PDU and unlock a potential wait queue */
old_state = ISOTP_IDLE; so->tx.state = ISOTP_IDLE;
err_out: wake_up_interruptible(&so->wait);
so->tx.state = old_state;
if (so->tx.state == ISOTP_IDLE)
wake_up_interruptible(&so->wait);
return err; return err;
} }
...@@ -1120,7 +1125,7 @@ static int isotp_recvmsg(struct socket *sock, struct msghdr *msg, size_t size, ...@@ -1120,7 +1125,7 @@ static int isotp_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
if (ret < 0) if (ret < 0)
goto out_err; goto out_err;
sock_recv_timestamp(msg, sk, skb); sock_recv_cmsgs(msg, sk, skb);
if (msg->msg_name) { if (msg->msg_name) {
__sockaddr_check_size(ISOTP_MIN_NAMELEN); __sockaddr_check_size(ISOTP_MIN_NAMELEN);
...@@ -1150,10 +1155,12 @@ static int isotp_release(struct socket *sock) ...@@ -1150,10 +1155,12 @@ static int isotp_release(struct socket *sock)
net = sock_net(sk); net = sock_net(sk);
/* wait for complete transmission of current pdu */ /* wait for complete transmission of current pdu */
wait_event_interruptible(so->wait, so->tx.state == ISOTP_IDLE); while (wait_event_interruptible(so->wait, so->tx.state == ISOTP_IDLE) == 0 &&
cmpxchg(&so->tx.state, ISOTP_IDLE, ISOTP_SHUTDOWN) != ISOTP_IDLE)
;
/* force state machines to be idle also when a signal occurred */ /* force state machines to be idle also when a signal occurred */
so->tx.state = ISOTP_IDLE; so->tx.state = ISOTP_SHUTDOWN;
so->rx.state = ISOTP_IDLE; so->rx.state = ISOTP_IDLE;
spin_lock(&isotp_notifier_lock); spin_lock(&isotp_notifier_lock);
...@@ -1608,6 +1615,21 @@ static int isotp_init(struct sock *sk) ...@@ -1608,6 +1615,21 @@ static int isotp_init(struct sock *sk)
return 0; return 0;
} }
static __poll_t isotp_poll(struct file *file, struct socket *sock, poll_table *wait)
{
struct sock *sk = sock->sk;
struct isotp_sock *so = isotp_sk(sk);
__poll_t mask = datagram_poll(file, sock, wait);
poll_wait(file, &so->wait, wait);
/* Check for false positives due to TX state */
if ((mask & EPOLLWRNORM) && (so->tx.state != ISOTP_IDLE))
mask &= ~(EPOLLOUT | EPOLLWRNORM);
return mask;
}
static int isotp_sock_no_ioctlcmd(struct socket *sock, unsigned int cmd, static int isotp_sock_no_ioctlcmd(struct socket *sock, unsigned int cmd,
unsigned long arg) unsigned long arg)
{ {
...@@ -1623,7 +1645,7 @@ static const struct proto_ops isotp_ops = { ...@@ -1623,7 +1645,7 @@ static const struct proto_ops isotp_ops = {
.socketpair = sock_no_socketpair, .socketpair = sock_no_socketpair,
.accept = sock_no_accept, .accept = sock_no_accept,
.getname = isotp_getname, .getname = isotp_getname,
.poll = datagram_poll, .poll = isotp_poll,
.ioctl = isotp_sock_no_ioctlcmd, .ioctl = isotp_sock_no_ioctlcmd,
.gettstamp = sock_gettstamp, .gettstamp = sock_gettstamp,
.listen = sock_no_listen, .listen = sock_no_listen,
......
...@@ -604,7 +604,10 @@ sk_buff *j1939_tp_tx_dat_new(struct j1939_priv *priv, ...@@ -604,7 +604,10 @@ sk_buff *j1939_tp_tx_dat_new(struct j1939_priv *priv,
/* reserve CAN header */ /* reserve CAN header */
skb_reserve(skb, offsetof(struct can_frame, data)); skb_reserve(skb, offsetof(struct can_frame, data));
memcpy(skb->cb, re_skcb, sizeof(skb->cb)); /* skb->cb must be large enough to hold a j1939_sk_buff_cb structure */
BUILD_BUG_ON(sizeof(skb->cb) < sizeof(*re_skcb));
memcpy(skb->cb, re_skcb, sizeof(*re_skcb));
skcb = j1939_skb_to_cb(skb); skcb = j1939_skb_to_cb(skb);
if (swap_src_dst) if (swap_src_dst)
j1939_skbcb_swap(skcb); j1939_skbcb_swap(skcb);
......
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