* [PATCH v2 1/2] ax25: Fix use of copy_from_sockptr() in ax25_setsockopt()
@ 2021-10-12 20:05 Ralf Baechle
2021-10-12 20:05 ` [PATCH 2/2] ax25: Fix deadlock hang during concurrent read and write on socket Ralf Baechle
0 siblings, 1 reply; 4+ messages in thread
From: Ralf Baechle @ 2021-10-12 20:05 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Jakub Kicinski, Christoph Hellwig,
Thomas Osterried, linux-hams
The destination pointer passed to copy_from_sockptr() is an unsigned long *
but the source in userspace is an unsigned int.
This happens to work on 32 bit but breaks 64-bit where bytes 4..7 will not
be initialized. By luck it may work on little endian but on big endian
where the userspace data is copied to the upper 32 bit of the destination
it's most likely going to break.
Signed-off-by: Ralf Baechle <ralf@linux-mips.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Fixes: a7b75c5a8c41 ("net: pass a sockptr_t into ->setsockopt")
---
net/ax25/af_ax25.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c
index 2631efc6e359..5e7ab76f7f9b 100644
--- a/net/ax25/af_ax25.c
+++ b/net/ax25/af_ax25.c
@@ -534,7 +534,7 @@ static int ax25_setsockopt(struct socket *sock, int level, int optname,
ax25_cb *ax25;
struct net_device *dev;
char devname[IFNAMSIZ];
- unsigned long opt;
+ unsigned int opt;
int res = 0;
if (level != SOL_AX25)
@@ -566,7 +566,7 @@ static int ax25_setsockopt(struct socket *sock, int level, int optname,
break;
case AX25_T1:
- if (opt < 1 || opt > ULONG_MAX / HZ) {
+ if (opt < 1 || opt > UINT_MAX / HZ) {
res = -EINVAL;
break;
}
@@ -575,7 +575,7 @@ static int ax25_setsockopt(struct socket *sock, int level, int optname,
break;
case AX25_T2:
- if (opt < 1 || opt > ULONG_MAX / HZ) {
+ if (opt < 1 || opt > UINT_MAX / HZ) {
res = -EINVAL;
break;
}
@@ -591,7 +591,7 @@ static int ax25_setsockopt(struct socket *sock, int level, int optname,
break;
case AX25_T3:
- if (opt < 1 || opt > ULONG_MAX / HZ) {
+ if (opt < 1 || opt > UINT_MAX / HZ) {
res = -EINVAL;
break;
}
@@ -599,7 +599,7 @@ static int ax25_setsockopt(struct socket *sock, int level, int optname,
break;
case AX25_IDLE:
- if (opt > ULONG_MAX / (60 * HZ)) {
+ if (opt > UINT_MAX / (60 * HZ)) {
res = -EINVAL;
break;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH 2/2] ax25: Fix deadlock hang during concurrent read and write on socket.
2021-10-12 20:05 [PATCH v2 1/2] ax25: Fix use of copy_from_sockptr() in ax25_setsockopt() Ralf Baechle
@ 2021-10-12 20:05 ` Ralf Baechle
2021-10-12 22:09 ` Jakub Kicinski
2021-10-13 7:31 ` kernel test robot
0 siblings, 2 replies; 4+ messages in thread
From: Ralf Baechle @ 2021-10-12 20:05 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Jakub Kicinski, Christoph Hellwig,
Thomas Osterried, linux-hams
From: Thomas Habets <thomas@habets.se>
Before this patch, this hangs, because the read(2) blocks the
write(2).
Before:
strace -f -eread,write ./examples/client_lockcheck M0THC-9 M0THC-0 M0THC-2
strace: Process 3888 attached
[pid 3888] read(3, <unfinished ...>
[pid 3887] write(3, "hello world", 11
[hang]
After:
strace -f -eread,write ./examples/client_lockcheck M0THC-9 M0THC-0 M0THC-2
strace: Process 2433 attached
[pid 2433] read(3, <unfinished ...>
[pid 2432] write(3, "hello world", 11) = 11
[pid 2433] <... read resumed> "yo", 1000) = 2
[pid 2433] write(1, "yo\n", 3yo
) = 3
[successful exit]
Signed-off-by: Thomas Habets <thomas@habets.se>
Signed-off-by: Ralf Baechle <ralf@linux-mips.org>
---
net/ax25/af_ax25.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c
index 5e7ab76f7f9b..d2d0dd744bb4 100644
--- a/net/ax25/af_ax25.c
+++ b/net/ax25/af_ax25.c
@@ -1624,22 +1624,22 @@ static int ax25_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
int copied;
int err = 0;
- lock_sock(sk);
/*
* This works for seqpacket too. The receiver has ordered the
* queue for us! We do one quick check first though
*/
if (sk->sk_type == SOCK_SEQPACKET && sk->sk_state != TCP_ESTABLISHED) {
err = -ENOTCONN;
- goto out;
+ goto out_nolock;
}
/* Now we can treat all alike */
skb = skb_recv_datagram(sk, flags & ~MSG_DONTWAIT,
flags & MSG_DONTWAIT, &err);
if (skb == NULL)
- goto out;
+ goto out_nolock;
+ lock_sock(sk);
if (!sk_to_ax25(sk)->pidincl)
skb_pull(skb, 1); /* Remove PID */
@@ -1684,6 +1684,7 @@ static int ax25_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
out:
release_sock(sk);
+out_nolock:
return err;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH 2/2] ax25: Fix deadlock hang during concurrent read and write on socket.
2021-10-12 20:05 ` [PATCH 2/2] ax25: Fix deadlock hang during concurrent read and write on socket Ralf Baechle
@ 2021-10-12 22:09 ` Jakub Kicinski
2021-10-13 7:31 ` kernel test robot
1 sibling, 0 replies; 4+ messages in thread
From: Jakub Kicinski @ 2021-10-12 22:09 UTC (permalink / raw)
To: Ralf Baechle
Cc: netdev, David S. Miller, Christoph Hellwig, Thomas Osterried,
linux-hams
On Tue, 12 Oct 2021 22:05:30 +0200 Ralf Baechle wrote:
> From: Thomas Habets <thomas@habets.se>
>
> Before this patch, this hangs, because the read(2) blocks the
> write(2).
Still build issues:
net/ax25/af_ax25.c: In function ‘ax25_recvmsg’:
net/ax25/af_ax25.c:1685:1: warning: label ‘out’ defined but not used [-Wunused-label]
1685 | out:
| ^~~
net/ax25/af_ax25.c:1685:1: warning: unused label 'out'
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH 2/2] ax25: Fix deadlock hang during concurrent read and write on socket.
2021-10-12 20:05 ` [PATCH 2/2] ax25: Fix deadlock hang during concurrent read and write on socket Ralf Baechle
2021-10-12 22:09 ` Jakub Kicinski
@ 2021-10-13 7:31 ` kernel test robot
1 sibling, 0 replies; 4+ messages in thread
From: kernel test robot @ 2021-10-13 7:31 UTC (permalink / raw)
To: Ralf Baechle, netdev
Cc: llvm, kbuild-all, Jakub Kicinski, Christoph Hellwig,
Thomas Osterried, linux-hams
[-- Attachment #1: Type: text/plain, Size: 7963 bytes --]
Hi Ralf,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on net/master]
[also build test WARNING on net-next/master horms-ipvs/master linus/master v5.15-rc5 next-20211012]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Ralf-Baechle/ax25-Fix-use-of-copy_from_sockptr-in-ax25_setsockopt/20211013-042226
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git 4d4a223a86afe658cd878800f09458e8bb54415d
config: x86_64-randconfig-a014-20211012 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project adf55ac6657693f7bfbe3087b599b4031a765a44)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/89cd241b1014e6501130d9116ea6ca367b10dc6a
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Ralf-Baechle/ax25-Fix-use-of-copy_from_sockptr-in-ax25_setsockopt/20211013-042226
git checkout 89cd241b1014e6501130d9116ea6ca367b10dc6a
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> net/ax25/af_ax25.c:1685:1: warning: unused label 'out' [-Wunused-label]
out:
^~~~
1 warning generated.
vim +/out +1685 net/ax25/af_ax25.c
^1da177e4c3f41 Linus Torvalds 2005-04-16 1618
1b784140474e4f Ying Xue 2015-03-02 1619 static int ax25_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
1b784140474e4f Ying Xue 2015-03-02 1620 int flags)
^1da177e4c3f41 Linus Torvalds 2005-04-16 1621 {
^1da177e4c3f41 Linus Torvalds 2005-04-16 1622 struct sock *sk = sock->sk;
^1da177e4c3f41 Linus Torvalds 2005-04-16 1623 struct sk_buff *skb;
^1da177e4c3f41 Linus Torvalds 2005-04-16 1624 int copied;
^1da177e4c3f41 Linus Torvalds 2005-04-16 1625 int err = 0;
^1da177e4c3f41 Linus Torvalds 2005-04-16 1626
^1da177e4c3f41 Linus Torvalds 2005-04-16 1627 /*
^1da177e4c3f41 Linus Torvalds 2005-04-16 1628 * This works for seqpacket too. The receiver has ordered the
^1da177e4c3f41 Linus Torvalds 2005-04-16 1629 * queue for us! We do one quick check first though
^1da177e4c3f41 Linus Torvalds 2005-04-16 1630 */
^1da177e4c3f41 Linus Torvalds 2005-04-16 1631 if (sk->sk_type == SOCK_SEQPACKET && sk->sk_state != TCP_ESTABLISHED) {
^1da177e4c3f41 Linus Torvalds 2005-04-16 1632 err = -ENOTCONN;
89cd241b1014e6 Thomas Habets 2021-10-12 1633 goto out_nolock;
^1da177e4c3f41 Linus Torvalds 2005-04-16 1634 }
^1da177e4c3f41 Linus Torvalds 2005-04-16 1635
^1da177e4c3f41 Linus Torvalds 2005-04-16 1636 /* Now we can treat all alike */
^1da177e4c3f41 Linus Torvalds 2005-04-16 1637 skb = skb_recv_datagram(sk, flags & ~MSG_DONTWAIT,
^1da177e4c3f41 Linus Torvalds 2005-04-16 1638 flags & MSG_DONTWAIT, &err);
^1da177e4c3f41 Linus Torvalds 2005-04-16 1639 if (skb == NULL)
89cd241b1014e6 Thomas Habets 2021-10-12 1640 goto out_nolock;
^1da177e4c3f41 Linus Torvalds 2005-04-16 1641
89cd241b1014e6 Thomas Habets 2021-10-12 1642 lock_sock(sk);
3200392b88dd25 David Miller 2015-06-25 1643 if (!sk_to_ax25(sk)->pidincl)
^1da177e4c3f41 Linus Torvalds 2005-04-16 1644 skb_pull(skb, 1); /* Remove PID */
^1da177e4c3f41 Linus Torvalds 2005-04-16 1645
badff6d01a8589 Arnaldo Carvalho de Melo 2007-03-13 1646 skb_reset_transport_header(skb);
^1da177e4c3f41 Linus Torvalds 2005-04-16 1647 copied = skb->len;
^1da177e4c3f41 Linus Torvalds 2005-04-16 1648
^1da177e4c3f41 Linus Torvalds 2005-04-16 1649 if (copied > size) {
^1da177e4c3f41 Linus Torvalds 2005-04-16 1650 copied = size;
^1da177e4c3f41 Linus Torvalds 2005-04-16 1651 msg->msg_flags |= MSG_TRUNC;
^1da177e4c3f41 Linus Torvalds 2005-04-16 1652 }
^1da177e4c3f41 Linus Torvalds 2005-04-16 1653
51f3d02b980a33 David S. Miller 2014-11-05 1654 skb_copy_datagram_msg(skb, 0, msg, copied);
^1da177e4c3f41 Linus Torvalds 2005-04-16 1655
f3d3342602f8bc Hannes Frederic Sowa 2013-11-21 1656 if (msg->msg_name) {
^1da177e4c3f41 Linus Torvalds 2005-04-16 1657 ax25_digi digi;
^1da177e4c3f41 Linus Torvalds 2005-04-16 1658 ax25_address src;
98e399f82ab3a6 Arnaldo Carvalho de Melo 2007-03-19 1659 const unsigned char *mac = skb_mac_header(skb);
342dfc306fb321 Steffen Hurrle 2014-01-17 1660 DECLARE_SOCKADDR(struct sockaddr_ax25 *, sax, msg->msg_name);
^1da177e4c3f41 Linus Torvalds 2005-04-16 1661
ef3313e84acbf3 Mathias Krause 2013-04-07 1662 memset(sax, 0, sizeof(struct full_sockaddr_ax25));
98e399f82ab3a6 Arnaldo Carvalho de Melo 2007-03-19 1663 ax25_addr_parse(mac + 1, skb->data - mac - 1, &src, NULL,
98e399f82ab3a6 Arnaldo Carvalho de Melo 2007-03-19 1664 &digi, NULL, NULL);
^1da177e4c3f41 Linus Torvalds 2005-04-16 1665 sax->sax25_family = AF_AX25;
^1da177e4c3f41 Linus Torvalds 2005-04-16 1666 /* We set this correctly, even though we may not let the
^1da177e4c3f41 Linus Torvalds 2005-04-16 1667 application know the digi calls further down (because it
^1da177e4c3f41 Linus Torvalds 2005-04-16 1668 did NOT ask to know them). This could get political... **/
^1da177e4c3f41 Linus Torvalds 2005-04-16 1669 sax->sax25_ndigis = digi.ndigi;
^1da177e4c3f41 Linus Torvalds 2005-04-16 1670 sax->sax25_call = src;
^1da177e4c3f41 Linus Torvalds 2005-04-16 1671
^1da177e4c3f41 Linus Torvalds 2005-04-16 1672 if (sax->sax25_ndigis != 0) {
^1da177e4c3f41 Linus Torvalds 2005-04-16 1673 int ct;
^1da177e4c3f41 Linus Torvalds 2005-04-16 1674 struct full_sockaddr_ax25 *fsa = (struct full_sockaddr_ax25 *)sax;
^1da177e4c3f41 Linus Torvalds 2005-04-16 1675
^1da177e4c3f41 Linus Torvalds 2005-04-16 1676 for (ct = 0; ct < digi.ndigi; ct++)
^1da177e4c3f41 Linus Torvalds 2005-04-16 1677 fsa->fsa_digipeater[ct] = digi.calls[ct];
^1da177e4c3f41 Linus Torvalds 2005-04-16 1678 }
^1da177e4c3f41 Linus Torvalds 2005-04-16 1679 msg->msg_namelen = sizeof(struct full_sockaddr_ax25);
^1da177e4c3f41 Linus Torvalds 2005-04-16 1680 }
^1da177e4c3f41 Linus Torvalds 2005-04-16 1681
^1da177e4c3f41 Linus Torvalds 2005-04-16 1682 skb_free_datagram(sk, skb);
^1da177e4c3f41 Linus Torvalds 2005-04-16 1683 err = copied;
^1da177e4c3f41 Linus Torvalds 2005-04-16 1684
^1da177e4c3f41 Linus Torvalds 2005-04-16 @1685 out:
^1da177e4c3f41 Linus Torvalds 2005-04-16 1686 release_sock(sk);
89cd241b1014e6 Thomas Habets 2021-10-12 1687 out_nolock:
^1da177e4c3f41 Linus Torvalds 2005-04-16 1688
^1da177e4c3f41 Linus Torvalds 2005-04-16 1689 return err;
^1da177e4c3f41 Linus Torvalds 2005-04-16 1690 }
^1da177e4c3f41 Linus Torvalds 2005-04-16 1691
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 36329 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-10-13 7:31 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-10-12 20:05 [PATCH v2 1/2] ax25: Fix use of copy_from_sockptr() in ax25_setsockopt() Ralf Baechle
2021-10-12 20:05 ` [PATCH 2/2] ax25: Fix deadlock hang during concurrent read and write on socket Ralf Baechle
2021-10-12 22:09 ` Jakub Kicinski
2021-10-13 7:31 ` kernel test robot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).