All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: kbuild@lists.01.org, Arjun Roy <arjunroy.kdev@gmail.com>,
	davem@davemloft.net, netdev@vger.kernel.org
Cc: lkp@intel.com, kbuild-all@lists.01.org, arjunroy@google.com,
	edumazet@google.com, soheil@google.com,
	David Ahern <dsahern@gmail.com>,
	Leon Romanovsky <leon@kernel.org>,
	Jakub Kicinski <kuba@kernel.org>
Subject: Re: [net-next] tcp: Sanitize CMSG flags and reserved args in tcp_zerocopy_receive.
Date: Mon, 15 Feb 2021 15:03:45 +0300	[thread overview]
Message-ID: <20210215120345.GE2087@kadam> (raw)
In-Reply-To: <20210211212107.662291-1-arjunroy.kdev@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 3441 bytes --]

Hi Arjun,

url:    https://github.com/0day-ci/linux/commits/Arjun-Roy/tcp-Sanitize-CMSG-flags-and-reserved-args-in-tcp_zerocopy_receive/20210212-052537
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git e4b62cf7559f2ef9a022de235e5a09a8d7ded520
config: x86_64-randconfig-m001-20210209 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

smatch warnings:
net/ipv4/tcp.c:4158 do_tcp_getsockopt() warn: check for integer overflow 'len'

vim +/len +4158 net/ipv4/tcp.c

3fdadf7d27e3fb Dmitry Mishin            2006-03-20  3896  static int do_tcp_getsockopt(struct sock *sk, int level,
3fdadf7d27e3fb Dmitry Mishin            2006-03-20  3897  		int optname, char __user *optval, int __user *optlen)
^1da177e4c3f41 Linus Torvalds           2005-04-16  3898  {
295f7324ff8d9e Arnaldo Carvalho de Melo 2005-08-09  3899  	struct inet_connection_sock *icsk = inet_csk(sk);
^1da177e4c3f41 Linus Torvalds           2005-04-16  3900  	struct tcp_sock *tp = tcp_sk(sk);
6fa251663069e0 Nikolay Borisov          2016-02-03  3901  	struct net *net = sock_net(sk);
^1da177e4c3f41 Linus Torvalds           2005-04-16  3902  	int val, len;

"len" is int.

[ snip ]
05255b823a6173 Eric Dumazet             2018-04-27  4146  #ifdef CONFIG_MMU
05255b823a6173 Eric Dumazet             2018-04-27  4147  	case TCP_ZEROCOPY_RECEIVE: {
7eeba1706eba6d Arjun Roy                2021-01-20  4148  		struct scm_timestamping_internal tss;
e0fecb289ad3fd Arjun Roy                2020-12-10  4149  		struct tcp_zerocopy_receive zc = {};
05255b823a6173 Eric Dumazet             2018-04-27  4150  		int err;
05255b823a6173 Eric Dumazet             2018-04-27  4151  
05255b823a6173 Eric Dumazet             2018-04-27  4152  		if (get_user(len, optlen))
05255b823a6173 Eric Dumazet             2018-04-27  4153  			return -EFAULT;
c8856c05145490 Arjun Roy                2020-02-14  4154  		if (len < offsetofend(struct tcp_zerocopy_receive, length))
05255b823a6173 Eric Dumazet             2018-04-27  4155  			return -EINVAL;


The problem is that negative values of "len" are type promoted to high
positive values.  So the fix is to write this as:

	if (len < 0 || len < offsetofend(struct tcp_zerocopy_receive, length))
		return -EINVAL;

110912bdf28392 Arjun Roy                2021-02-11  4156  		if (unlikely(len > sizeof(zc))) {
110912bdf28392 Arjun Roy                2021-02-11  4157  			err = check_zeroed_user(optval + sizeof(zc),
110912bdf28392 Arjun Roy                2021-02-11 @4158  						len - sizeof(zc));
                                                                                                        ^^^^^^^^^^^^^^^^
Potentially "len - a negative value".


110912bdf28392 Arjun Roy                2021-02-11  4159  			if (err < 1)
110912bdf28392 Arjun Roy                2021-02-11  4160  				return err == 0 ? -EINVAL : err;
c8856c05145490 Arjun Roy                2020-02-14  4161  			len = sizeof(zc);
0b7f41f68710cc Arjun Roy                2020-02-25  4162  			if (put_user(len, optlen))
0b7f41f68710cc Arjun Roy                2020-02-25  4163  				return -EFAULT;
0b7f41f68710cc Arjun Roy                2020-02-25  4164  		}

---
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: 29600 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: Dan Carpenter <dan.carpenter@oracle.com>
To: kbuild@lists.01.org
Subject: Re: [net-next] tcp: Sanitize CMSG flags and reserved args in tcp_zerocopy_receive.
Date: Mon, 15 Feb 2021 15:03:45 +0300	[thread overview]
Message-ID: <20210215120345.GE2087@kadam> (raw)
In-Reply-To: <20210211212107.662291-1-arjunroy.kdev@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 3503 bytes --]

Hi Arjun,

url:    https://github.com/0day-ci/linux/commits/Arjun-Roy/tcp-Sanitize-CMSG-flags-and-reserved-args-in-tcp_zerocopy_receive/20210212-052537
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git e4b62cf7559f2ef9a022de235e5a09a8d7ded520
config: x86_64-randconfig-m001-20210209 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

smatch warnings:
net/ipv4/tcp.c:4158 do_tcp_getsockopt() warn: check for integer overflow 'len'

vim +/len +4158 net/ipv4/tcp.c

3fdadf7d27e3fb Dmitry Mishin            2006-03-20  3896  static int do_tcp_getsockopt(struct sock *sk, int level,
3fdadf7d27e3fb Dmitry Mishin            2006-03-20  3897  		int optname, char __user *optval, int __user *optlen)
^1da177e4c3f41 Linus Torvalds           2005-04-16  3898  {
295f7324ff8d9e Arnaldo Carvalho de Melo 2005-08-09  3899  	struct inet_connection_sock *icsk = inet_csk(sk);
^1da177e4c3f41 Linus Torvalds           2005-04-16  3900  	struct tcp_sock *tp = tcp_sk(sk);
6fa251663069e0 Nikolay Borisov          2016-02-03  3901  	struct net *net = sock_net(sk);
^1da177e4c3f41 Linus Torvalds           2005-04-16  3902  	int val, len;

"len" is int.

[ snip ]
05255b823a6173 Eric Dumazet             2018-04-27  4146  #ifdef CONFIG_MMU
05255b823a6173 Eric Dumazet             2018-04-27  4147  	case TCP_ZEROCOPY_RECEIVE: {
7eeba1706eba6d Arjun Roy                2021-01-20  4148  		struct scm_timestamping_internal tss;
e0fecb289ad3fd Arjun Roy                2020-12-10  4149  		struct tcp_zerocopy_receive zc = {};
05255b823a6173 Eric Dumazet             2018-04-27  4150  		int err;
05255b823a6173 Eric Dumazet             2018-04-27  4151  
05255b823a6173 Eric Dumazet             2018-04-27  4152  		if (get_user(len, optlen))
05255b823a6173 Eric Dumazet             2018-04-27  4153  			return -EFAULT;
c8856c05145490 Arjun Roy                2020-02-14  4154  		if (len < offsetofend(struct tcp_zerocopy_receive, length))
05255b823a6173 Eric Dumazet             2018-04-27  4155  			return -EINVAL;


The problem is that negative values of "len" are type promoted to high
positive values.  So the fix is to write this as:

	if (len < 0 || len < offsetofend(struct tcp_zerocopy_receive, length))
		return -EINVAL;

110912bdf28392 Arjun Roy                2021-02-11  4156  		if (unlikely(len > sizeof(zc))) {
110912bdf28392 Arjun Roy                2021-02-11  4157  			err = check_zeroed_user(optval + sizeof(zc),
110912bdf28392 Arjun Roy                2021-02-11 @4158  						len - sizeof(zc));
                                                                                                        ^^^^^^^^^^^^^^^^
Potentially "len - a negative value".


110912bdf28392 Arjun Roy                2021-02-11  4159  			if (err < 1)
110912bdf28392 Arjun Roy                2021-02-11  4160  				return err == 0 ? -EINVAL : err;
c8856c05145490 Arjun Roy                2020-02-14  4161  			len = sizeof(zc);
0b7f41f68710cc Arjun Roy                2020-02-25  4162  			if (put_user(len, optlen))
0b7f41f68710cc Arjun Roy                2020-02-25  4163  				return -EFAULT;
0b7f41f68710cc Arjun Roy                2020-02-25  4164  		}

---
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: 29600 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: Dan Carpenter <dan.carpenter@oracle.com>
To: kbuild-all@lists.01.org
Subject: Re: [net-next] tcp: Sanitize CMSG flags and reserved args in tcp_zerocopy_receive.
Date: Mon, 15 Feb 2021 15:03:45 +0300	[thread overview]
Message-ID: <20210215120345.GE2087@kadam> (raw)
In-Reply-To: <20210211212107.662291-1-arjunroy.kdev@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 3503 bytes --]

Hi Arjun,

url:    https://github.com/0day-ci/linux/commits/Arjun-Roy/tcp-Sanitize-CMSG-flags-and-reserved-args-in-tcp_zerocopy_receive/20210212-052537
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git e4b62cf7559f2ef9a022de235e5a09a8d7ded520
config: x86_64-randconfig-m001-20210209 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

smatch warnings:
net/ipv4/tcp.c:4158 do_tcp_getsockopt() warn: check for integer overflow 'len'

vim +/len +4158 net/ipv4/tcp.c

3fdadf7d27e3fb Dmitry Mishin            2006-03-20  3896  static int do_tcp_getsockopt(struct sock *sk, int level,
3fdadf7d27e3fb Dmitry Mishin            2006-03-20  3897  		int optname, char __user *optval, int __user *optlen)
^1da177e4c3f41 Linus Torvalds           2005-04-16  3898  {
295f7324ff8d9e Arnaldo Carvalho de Melo 2005-08-09  3899  	struct inet_connection_sock *icsk = inet_csk(sk);
^1da177e4c3f41 Linus Torvalds           2005-04-16  3900  	struct tcp_sock *tp = tcp_sk(sk);
6fa251663069e0 Nikolay Borisov          2016-02-03  3901  	struct net *net = sock_net(sk);
^1da177e4c3f41 Linus Torvalds           2005-04-16  3902  	int val, len;

"len" is int.

[ snip ]
05255b823a6173 Eric Dumazet             2018-04-27  4146  #ifdef CONFIG_MMU
05255b823a6173 Eric Dumazet             2018-04-27  4147  	case TCP_ZEROCOPY_RECEIVE: {
7eeba1706eba6d Arjun Roy                2021-01-20  4148  		struct scm_timestamping_internal tss;
e0fecb289ad3fd Arjun Roy                2020-12-10  4149  		struct tcp_zerocopy_receive zc = {};
05255b823a6173 Eric Dumazet             2018-04-27  4150  		int err;
05255b823a6173 Eric Dumazet             2018-04-27  4151  
05255b823a6173 Eric Dumazet             2018-04-27  4152  		if (get_user(len, optlen))
05255b823a6173 Eric Dumazet             2018-04-27  4153  			return -EFAULT;
c8856c05145490 Arjun Roy                2020-02-14  4154  		if (len < offsetofend(struct tcp_zerocopy_receive, length))
05255b823a6173 Eric Dumazet             2018-04-27  4155  			return -EINVAL;


The problem is that negative values of "len" are type promoted to high
positive values.  So the fix is to write this as:

	if (len < 0 || len < offsetofend(struct tcp_zerocopy_receive, length))
		return -EINVAL;

110912bdf28392 Arjun Roy                2021-02-11  4156  		if (unlikely(len > sizeof(zc))) {
110912bdf28392 Arjun Roy                2021-02-11  4157  			err = check_zeroed_user(optval + sizeof(zc),
110912bdf28392 Arjun Roy                2021-02-11 @4158  						len - sizeof(zc));
                                                                                                        ^^^^^^^^^^^^^^^^
Potentially "len - a negative value".


110912bdf28392 Arjun Roy                2021-02-11  4159  			if (err < 1)
110912bdf28392 Arjun Roy                2021-02-11  4160  				return err == 0 ? -EINVAL : err;
c8856c05145490 Arjun Roy                2020-02-14  4161  			len = sizeof(zc);
0b7f41f68710cc Arjun Roy                2020-02-25  4162  			if (put_user(len, optlen))
0b7f41f68710cc Arjun Roy                2020-02-25  4163  				return -EFAULT;
0b7f41f68710cc Arjun Roy                2020-02-25  4164  		}

---
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: 29600 bytes --]

  parent reply	other threads:[~2021-02-15 12:05 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-11 21:21 [net-next] tcp: Sanitize CMSG flags and reserved args in tcp_zerocopy_receive Arjun Roy
2021-02-12  2:08 ` Jakub Kicinski
2021-02-12  3:10 ` patchwork-bot+netdevbpf
2021-02-15 12:03 ` Dan Carpenter [this message]
2021-02-15 12:03   ` Dan Carpenter
2021-02-15 12:03   ` Dan Carpenter
2021-02-15 15:04   ` David Ahern
2021-02-15 16:02     ` Dan Carpenter
2021-02-15 16:02       ` Dan Carpenter
2021-02-15 16:02       ` Dan Carpenter
2021-02-25 22:59       ` Arjun Roy
2021-02-25 23:00       ` Arjun Roy
2021-02-25 23:00         ` Arjun Roy
2021-02-12  2:28 kernel test robot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210215120345.GE2087@kadam \
    --to=dan.carpenter@oracle.com \
    --cc=arjunroy.kdev@gmail.com \
    --cc=arjunroy@google.com \
    --cc=davem@davemloft.net \
    --cc=dsahern@gmail.com \
    --cc=edumazet@google.com \
    --cc=kbuild-all@lists.01.org \
    --cc=kbuild@lists.01.org \
    --cc=kuba@kernel.org \
    --cc=leon@kernel.org \
    --cc=lkp@intel.com \
    --cc=netdev@vger.kernel.org \
    --cc=soheil@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.