* [PATCH] ax25: Fix use of copy_from_sockptr() in ax25_setsockopt()
@ 2021-09-30 16:24 Ralf Baechle
2021-10-01 2:31 ` kernel test robot
2021-10-12 6:23 ` Christoph Hellwig
0 siblings, 2 replies; 4+ messages in thread
From: Ralf Baechle @ 2021-09-30 16:24 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 * resulting in an integer
of the wrong size being copied from userspace.
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.
A simple test case to demonstrate this setsockopt() issue is:
[...]
int sk = socket(AF_AX25, SOCK_SEQPACKET, 0);
int n1 = 42;
int res = setsockopt(sk, SOL_AX25, AX25_T1, &n1, sizeof(n1));
printf("res = %d\n", res);
[...]
Signed-off-by: Ralf Baechle <ralf@linux-mips.org>
Cc: stable@vger.kernel.org # 5.9
Fixes: a7b75c5a8c41 ("net: pass a sockptr_t into ->setsockopt")
---
net/ax25/af_ax25.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c
index 2631efc6e359..9f2e4b76394a 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)
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] ax25: Fix use of copy_from_sockptr() in ax25_setsockopt()
2021-09-30 16:24 [PATCH] ax25: Fix use of copy_from_sockptr() in ax25_setsockopt() Ralf Baechle
@ 2021-10-01 2:31 ` kernel test robot
2021-10-12 6:23 ` Christoph Hellwig
1 sibling, 0 replies; 4+ messages in thread
From: kernel test robot @ 2021-10-01 2: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: 13453 bytes --]
Hi Ralf,
I love your 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-rc3 next-20210922]
[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/20211001-002911
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git 35306eb23814444bd4021f8a1c3047d3cb0c8b2b
config: x86_64-randconfig-r012-20210930 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 962e503cc8bc411f7523cc393acae8aae425b1c4)
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/a84e2204c4c268dd4475a95ed472285982ae6c57
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/20211001-002911
git checkout a84e2204c4c268dd4475a95ed472285982ae6c57
# 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:569:22: warning: result of comparison of constant 18446744073709551 with expression of type 'unsigned int' is always false [-Wtautological-constant-out-of-range-compare]
if (opt < 1 || opt > ULONG_MAX / HZ) {
~~~ ^ ~~~~~~~~~~~~~~
net/ax25/af_ax25.c:578:22: warning: result of comparison of constant 18446744073709551 with expression of type 'unsigned int' is always false [-Wtautological-constant-out-of-range-compare]
if (opt < 1 || opt > ULONG_MAX / HZ) {
~~~ ^ ~~~~~~~~~~~~~~
net/ax25/af_ax25.c:594:22: warning: result of comparison of constant 18446744073709551 with expression of type 'unsigned int' is always false [-Wtautological-constant-out-of-range-compare]
if (opt < 1 || opt > ULONG_MAX / HZ) {
~~~ ^ ~~~~~~~~~~~~~~
net/ax25/af_ax25.c:602:11: warning: result of comparison of constant 307445734561825 with expression of type 'unsigned int' is always false [-Wtautological-constant-out-of-range-compare]
if (opt > ULONG_MAX / (60 * HZ)) {
~~~ ^ ~~~~~~~~~~~~~~~~~~~~~
4 warnings generated.
vim +569 net/ax25/af_ax25.c
^1da177e4c3f41 Linus Torvalds 2005-04-16 524
^1da177e4c3f41 Linus Torvalds 2005-04-16 525 /*
^1da177e4c3f41 Linus Torvalds 2005-04-16 526 * Handling for system calls applied via the various interfaces to an
^1da177e4c3f41 Linus Torvalds 2005-04-16 527 * AX25 socket object
^1da177e4c3f41 Linus Torvalds 2005-04-16 528 */
^1da177e4c3f41 Linus Torvalds 2005-04-16 529
^1da177e4c3f41 Linus Torvalds 2005-04-16 530 static int ax25_setsockopt(struct socket *sock, int level, int optname,
a7b75c5a8c4144 Christoph Hellwig 2020-07-23 531 sockptr_t optval, unsigned int optlen)
^1da177e4c3f41 Linus Torvalds 2005-04-16 532 {
^1da177e4c3f41 Linus Torvalds 2005-04-16 533 struct sock *sk = sock->sk;
^1da177e4c3f41 Linus Torvalds 2005-04-16 534 ax25_cb *ax25;
^1da177e4c3f41 Linus Torvalds 2005-04-16 535 struct net_device *dev;
^1da177e4c3f41 Linus Torvalds 2005-04-16 536 char devname[IFNAMSIZ];
a84e2204c4c268 Ralf Baechle 2021-09-30 537 unsigned int opt;
ba1cffe0257bcd Xi Wang 2011-12-27 538 int res = 0;
^1da177e4c3f41 Linus Torvalds 2005-04-16 539
^1da177e4c3f41 Linus Torvalds 2005-04-16 540 if (level != SOL_AX25)
^1da177e4c3f41 Linus Torvalds 2005-04-16 541 return -ENOPROTOOPT;
^1da177e4c3f41 Linus Torvalds 2005-04-16 542
ba1cffe0257bcd Xi Wang 2011-12-27 543 if (optlen < sizeof(unsigned int))
^1da177e4c3f41 Linus Torvalds 2005-04-16 544 return -EINVAL;
^1da177e4c3f41 Linus Torvalds 2005-04-16 545
a7b75c5a8c4144 Christoph Hellwig 2020-07-23 546 if (copy_from_sockptr(&opt, optval, sizeof(unsigned int)))
^1da177e4c3f41 Linus Torvalds 2005-04-16 547 return -EFAULT;
^1da177e4c3f41 Linus Torvalds 2005-04-16 548
^1da177e4c3f41 Linus Torvalds 2005-04-16 549 lock_sock(sk);
3200392b88dd25 David Miller 2015-06-25 550 ax25 = sk_to_ax25(sk);
^1da177e4c3f41 Linus Torvalds 2005-04-16 551
^1da177e4c3f41 Linus Torvalds 2005-04-16 552 switch (optname) {
^1da177e4c3f41 Linus Torvalds 2005-04-16 553 case AX25_WINDOW:
^1da177e4c3f41 Linus Torvalds 2005-04-16 554 if (ax25->modulus == AX25_MODULUS) {
^1da177e4c3f41 Linus Torvalds 2005-04-16 555 if (opt < 1 || opt > 7) {
^1da177e4c3f41 Linus Torvalds 2005-04-16 556 res = -EINVAL;
^1da177e4c3f41 Linus Torvalds 2005-04-16 557 break;
^1da177e4c3f41 Linus Torvalds 2005-04-16 558 }
^1da177e4c3f41 Linus Torvalds 2005-04-16 559 } else {
^1da177e4c3f41 Linus Torvalds 2005-04-16 560 if (opt < 1 || opt > 63) {
^1da177e4c3f41 Linus Torvalds 2005-04-16 561 res = -EINVAL;
^1da177e4c3f41 Linus Torvalds 2005-04-16 562 break;
^1da177e4c3f41 Linus Torvalds 2005-04-16 563 }
^1da177e4c3f41 Linus Torvalds 2005-04-16 564 }
^1da177e4c3f41 Linus Torvalds 2005-04-16 565 ax25->window = opt;
^1da177e4c3f41 Linus Torvalds 2005-04-16 566 break;
^1da177e4c3f41 Linus Torvalds 2005-04-16 567
^1da177e4c3f41 Linus Torvalds 2005-04-16 568 case AX25_T1:
be639ac6901a08 Ralf Baechle 2011-11-24 @569 if (opt < 1 || opt > ULONG_MAX / HZ) {
^1da177e4c3f41 Linus Torvalds 2005-04-16 570 res = -EINVAL;
^1da177e4c3f41 Linus Torvalds 2005-04-16 571 break;
^1da177e4c3f41 Linus Torvalds 2005-04-16 572 }
f16f3026db6fa6 Eric Dumazet 2008-01-13 573 ax25->rtt = (opt * HZ) >> 1;
^1da177e4c3f41 Linus Torvalds 2005-04-16 574 ax25->t1 = opt * HZ;
^1da177e4c3f41 Linus Torvalds 2005-04-16 575 break;
^1da177e4c3f41 Linus Torvalds 2005-04-16 576
^1da177e4c3f41 Linus Torvalds 2005-04-16 577 case AX25_T2:
be639ac6901a08 Ralf Baechle 2011-11-24 578 if (opt < 1 || opt > ULONG_MAX / HZ) {
^1da177e4c3f41 Linus Torvalds 2005-04-16 579 res = -EINVAL;
^1da177e4c3f41 Linus Torvalds 2005-04-16 580 break;
^1da177e4c3f41 Linus Torvalds 2005-04-16 581 }
^1da177e4c3f41 Linus Torvalds 2005-04-16 582 ax25->t2 = opt * HZ;
^1da177e4c3f41 Linus Torvalds 2005-04-16 583 break;
^1da177e4c3f41 Linus Torvalds 2005-04-16 584
^1da177e4c3f41 Linus Torvalds 2005-04-16 585 case AX25_N2:
^1da177e4c3f41 Linus Torvalds 2005-04-16 586 if (opt < 1 || opt > 31) {
^1da177e4c3f41 Linus Torvalds 2005-04-16 587 res = -EINVAL;
^1da177e4c3f41 Linus Torvalds 2005-04-16 588 break;
^1da177e4c3f41 Linus Torvalds 2005-04-16 589 }
^1da177e4c3f41 Linus Torvalds 2005-04-16 590 ax25->n2 = opt;
^1da177e4c3f41 Linus Torvalds 2005-04-16 591 break;
^1da177e4c3f41 Linus Torvalds 2005-04-16 592
^1da177e4c3f41 Linus Torvalds 2005-04-16 593 case AX25_T3:
be639ac6901a08 Ralf Baechle 2011-11-24 594 if (opt < 1 || opt > ULONG_MAX / HZ) {
^1da177e4c3f41 Linus Torvalds 2005-04-16 595 res = -EINVAL;
^1da177e4c3f41 Linus Torvalds 2005-04-16 596 break;
^1da177e4c3f41 Linus Torvalds 2005-04-16 597 }
^1da177e4c3f41 Linus Torvalds 2005-04-16 598 ax25->t3 = opt * HZ;
^1da177e4c3f41 Linus Torvalds 2005-04-16 599 break;
^1da177e4c3f41 Linus Torvalds 2005-04-16 600
^1da177e4c3f41 Linus Torvalds 2005-04-16 601 case AX25_IDLE:
ba1cffe0257bcd Xi Wang 2011-12-27 602 if (opt > ULONG_MAX / (60 * HZ)) {
^1da177e4c3f41 Linus Torvalds 2005-04-16 603 res = -EINVAL;
^1da177e4c3f41 Linus Torvalds 2005-04-16 604 break;
^1da177e4c3f41 Linus Torvalds 2005-04-16 605 }
^1da177e4c3f41 Linus Torvalds 2005-04-16 606 ax25->idle = opt * 60 * HZ;
^1da177e4c3f41 Linus Torvalds 2005-04-16 607 break;
^1da177e4c3f41 Linus Torvalds 2005-04-16 608
^1da177e4c3f41 Linus Torvalds 2005-04-16 609 case AX25_BACKOFF:
ba1cffe0257bcd Xi Wang 2011-12-27 610 if (opt > 2) {
^1da177e4c3f41 Linus Torvalds 2005-04-16 611 res = -EINVAL;
^1da177e4c3f41 Linus Torvalds 2005-04-16 612 break;
^1da177e4c3f41 Linus Torvalds 2005-04-16 613 }
^1da177e4c3f41 Linus Torvalds 2005-04-16 614 ax25->backoff = opt;
^1da177e4c3f41 Linus Torvalds 2005-04-16 615 break;
^1da177e4c3f41 Linus Torvalds 2005-04-16 616
^1da177e4c3f41 Linus Torvalds 2005-04-16 617 case AX25_EXTSEQ:
^1da177e4c3f41 Linus Torvalds 2005-04-16 618 ax25->modulus = opt ? AX25_EMODULUS : AX25_MODULUS;
^1da177e4c3f41 Linus Torvalds 2005-04-16 619 break;
^1da177e4c3f41 Linus Torvalds 2005-04-16 620
^1da177e4c3f41 Linus Torvalds 2005-04-16 621 case AX25_PIDINCL:
^1da177e4c3f41 Linus Torvalds 2005-04-16 622 ax25->pidincl = opt ? 1 : 0;
^1da177e4c3f41 Linus Torvalds 2005-04-16 623 break;
^1da177e4c3f41 Linus Torvalds 2005-04-16 624
^1da177e4c3f41 Linus Torvalds 2005-04-16 625 case AX25_IAMDIGI:
^1da177e4c3f41 Linus Torvalds 2005-04-16 626 ax25->iamdigi = opt ? 1 : 0;
^1da177e4c3f41 Linus Torvalds 2005-04-16 627 break;
^1da177e4c3f41 Linus Torvalds 2005-04-16 628
^1da177e4c3f41 Linus Torvalds 2005-04-16 629 case AX25_PACLEN:
^1da177e4c3f41 Linus Torvalds 2005-04-16 630 if (opt < 16 || opt > 65535) {
^1da177e4c3f41 Linus Torvalds 2005-04-16 631 res = -EINVAL;
^1da177e4c3f41 Linus Torvalds 2005-04-16 632 break;
^1da177e4c3f41 Linus Torvalds 2005-04-16 633 }
^1da177e4c3f41 Linus Torvalds 2005-04-16 634 ax25->paclen = opt;
^1da177e4c3f41 Linus Torvalds 2005-04-16 635 break;
^1da177e4c3f41 Linus Torvalds 2005-04-16 636
^1da177e4c3f41 Linus Torvalds 2005-04-16 637 case SO_BINDTODEVICE:
687775cec056b3 Eric Dumazet 2020-05-19 638 if (optlen > IFNAMSIZ - 1)
687775cec056b3 Eric Dumazet 2020-05-19 639 optlen = IFNAMSIZ - 1;
687775cec056b3 Eric Dumazet 2020-05-19 640
687775cec056b3 Eric Dumazet 2020-05-19 641 memset(devname, 0, sizeof(devname));
2f72291d3d0e44 Ralf Baechle 2009-09-28 642
a7b75c5a8c4144 Christoph Hellwig 2020-07-23 643 if (copy_from_sockptr(devname, optval, optlen)) {
^1da177e4c3f41 Linus Torvalds 2005-04-16 644 res = -EFAULT;
^1da177e4c3f41 Linus Torvalds 2005-04-16 645 break;
^1da177e4c3f41 Linus Torvalds 2005-04-16 646 }
^1da177e4c3f41 Linus Torvalds 2005-04-16 647
^1da177e4c3f41 Linus Torvalds 2005-04-16 648 if (sk->sk_type == SOCK_SEQPACKET &&
^1da177e4c3f41 Linus Torvalds 2005-04-16 649 (sock->state != SS_UNCONNECTED ||
^1da177e4c3f41 Linus Torvalds 2005-04-16 650 sk->sk_state == TCP_LISTEN)) {
^1da177e4c3f41 Linus Torvalds 2005-04-16 651 res = -EADDRNOTAVAIL;
2f72291d3d0e44 Ralf Baechle 2009-09-28 652 break;
2f72291d3d0e44 Ralf Baechle 2009-09-28 653 }
2f72291d3d0e44 Ralf Baechle 2009-09-28 654
c433570458e49b Cong Wang 2018-12-29 655 rtnl_lock();
c433570458e49b Cong Wang 2018-12-29 656 dev = __dev_get_by_name(&init_net, devname);
2f72291d3d0e44 Ralf Baechle 2009-09-28 657 if (!dev) {
c433570458e49b Cong Wang 2018-12-29 658 rtnl_unlock();
2f72291d3d0e44 Ralf Baechle 2009-09-28 659 res = -ENODEV;
^1da177e4c3f41 Linus Torvalds 2005-04-16 660 break;
^1da177e4c3f41 Linus Torvalds 2005-04-16 661 }
^1da177e4c3f41 Linus Torvalds 2005-04-16 662
^1da177e4c3f41 Linus Torvalds 2005-04-16 663 ax25->ax25_dev = ax25_dev_ax25dev(dev);
c433570458e49b Cong Wang 2018-12-29 664 if (!ax25->ax25_dev) {
c433570458e49b Cong Wang 2018-12-29 665 rtnl_unlock();
c433570458e49b Cong Wang 2018-12-29 666 res = -ENODEV;
c433570458e49b Cong Wang 2018-12-29 667 break;
c433570458e49b Cong Wang 2018-12-29 668 }
^1da177e4c3f41 Linus Torvalds 2005-04-16 669 ax25_fillin_cb(ax25, ax25->ax25_dev);
c433570458e49b Cong Wang 2018-12-29 670 rtnl_unlock();
^1da177e4c3f41 Linus Torvalds 2005-04-16 671 break;
^1da177e4c3f41 Linus Torvalds 2005-04-16 672
^1da177e4c3f41 Linus Torvalds 2005-04-16 673 default:
^1da177e4c3f41 Linus Torvalds 2005-04-16 674 res = -ENOPROTOOPT;
^1da177e4c3f41 Linus Torvalds 2005-04-16 675 }
^1da177e4c3f41 Linus Torvalds 2005-04-16 676 release_sock(sk);
^1da177e4c3f41 Linus Torvalds 2005-04-16 677
^1da177e4c3f41 Linus Torvalds 2005-04-16 678 return res;
^1da177e4c3f41 Linus Torvalds 2005-04-16 679 }
^1da177e4c3f41 Linus Torvalds 2005-04-16 680
---
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: 34429 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] ax25: Fix use of copy_from_sockptr() in ax25_setsockopt()
2021-09-30 16:24 [PATCH] ax25: Fix use of copy_from_sockptr() in ax25_setsockopt() Ralf Baechle
2021-10-01 2:31 ` kernel test robot
@ 2021-10-12 6:23 ` Christoph Hellwig
2021-10-12 17:10 ` Ralf Baechle
1 sibling, 1 reply; 4+ messages in thread
From: Christoph Hellwig @ 2021-10-12 6:23 UTC (permalink / raw)
To: Ralf Baechle
Cc: netdev, David S. Miller, Jakub Kicinski, Christoph Hellwig,
Thomas Osterried, linux-hams
On Thu, Sep 30, 2021 at 06:24:35PM +0200, Ralf Baechle wrote:
> The destination pointer passed to copy_from_sockptr() is an unsigned long *
> but the source in userspace is an unsigned int * resulting in an integer
> of the wrong size being copied from userspace.
>
> 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.
>
> A simple test case to demonstrate this setsockopt() issue is:
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ax25: Fix use of copy_from_sockptr() in ax25_setsockopt()
2021-10-12 6:23 ` Christoph Hellwig
@ 2021-10-12 17:10 ` Ralf Baechle
0 siblings, 0 replies; 4+ messages in thread
From: Ralf Baechle @ 2021-10-12 17:10 UTC (permalink / raw)
To: Christoph Hellwig
Cc: netdev, David S. Miller, Jakub Kicinski, Thomas Osterried,
linux-hams
On Tue, Oct 12, 2021 at 08:23:09AM +0200, Christoph Hellwig wrote:
> On Thu, Sep 30, 2021 at 06:24:35PM +0200, Ralf Baechle wrote:
> > The destination pointer passed to copy_from_sockptr() is an unsigned long *
> > but the source in userspace is an unsigned int * resulting in an integer
> > of the wrong size being copied from userspace.
> >
> > 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.
> >
> > A simple test case to demonstrate this setsockopt() issue is:
>
> Looks good,
>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
Sadly the kernel test robot has raised a bunch of warnings in this patch.
To fix those I'll pull a few fixes from another patch I was planning to
send later and merge them into this patch and post the resulting patch
as v2.
Thanks for the review,
Ralf
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-10-12 17:10 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-09-30 16:24 [PATCH] ax25: Fix use of copy_from_sockptr() in ax25_setsockopt() Ralf Baechle
2021-10-01 2:31 ` kernel test robot
2021-10-12 6:23 ` Christoph Hellwig
2021-10-12 17:10 ` Ralf Baechle
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).