[PATCH net-next v2 0/6] fix incorrect parameter validation in the *_get_sockopt() functions

[PATCH net-next v2 0/6] fix incorrect parameter validation in the *_get_sockopt() functions

[Gavrilov Ilia]

This v2 series fix incorrent parameter validation in *_get_sockopt()
functions in several places.

version 2 changes:
- reword the patch description
- add two patches for net/kcm and net/x25


Gavrilov Ilia (6):
  tcp: fix incorrect parameter validation in the do_tcp_getsockopt()
    function@@
  udp: fix incorrect parameter validation in the udp_lib_getsockopt()
    function@@
  ipmr: fix incorrect parameter validation in the ip_mroute_getsockopt()
    function@@
  l2tp: fix incorrect parameter validation in the pppol2tp_getsockopt()
    function@@
  net: kcm: fix incorrect parameter validation in the kcm_getsockopt)
    function@@
  net/x25: fix incorrect parameter validation in the x25_getsockopt()
    function@@

 net/ipv4/ipmr.c     | 4 +++-
 net/ipv4/tcp.c      | 4 ++--
 net/ipv4/udp.c      | 4 ++--
 net/kcm/kcmsock.c   | 3 ++-
 net/l2tp/l2tp_ppp.c | 4 ++--
 net/x25/af_x25.c    | 4 ++--
 6 files changed, 13 insertions(+), 10 deletions(-)

-- 
2.39.2

[PATCH net-next v2 1/6] tcp: fix incorrect parameter validation in the do_tcp_getsockopt() function

[Gavrilov Ilia]

The 'len' variable can't be negative when assigned the result of
'min_t' because all 'min_t' parameters are cast to unsigned int,
and then the minimum one is chosen.

To fix the logic, check 'len' as read from 'optlen',
where the types of relevant variables are (signed) int.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Gavrilov Ilia <Ilia.Gavrilov@infotecs.ru>
---
V2:
 - reword the patch description

 net/ipv4/tcp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index c82dc42f57c6..a4f418592314 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -4010,11 +4010,11 @@ int do_tcp_getsockopt(struct sock *sk, int level,
 	if (copy_from_sockptr(&len, optlen, sizeof(int)))
 		return -EFAULT;
 
-	len = min_t(unsigned int, len, sizeof(int));
-
 	if (len < 0)
 		return -EINVAL;
 
+	len = min_t(unsigned int, len, sizeof(int));
+
 	switch (optname) {
 	case TCP_MAXSEG:
 		val = tp->mss_cache;
-- 
2.39.2

Re: [PATCH net-next v2 1/6] tcp: fix incorrect parameter validation in the do_tcp_getsockopt() function

[Jason Xing]

On Thu, Mar 7, 2024 at 10:44 PM Gavrilov Ilia <Ilia.Gavrilov@infotecs.ru> wrote:
>
> The 'len' variable can't be negative when assigned the result of
> 'min_t' because all 'min_t' parameters are cast to unsigned int,
> and then the minimum one is chosen.
>
> To fix the logic, check 'len' as read from 'optlen',
> where the types of relevant variables are (signed) int.
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Gavrilov Ilia <Ilia.Gavrilov@infotecs.ru>

For the patch itself, please feel free to add:
Reviewed-by: Jason Xing <kerneljasonxing@gmail.com>

I notice that you use Fixes meanwhile you target net-next. I'm not
sure if it's proper.

> ---
> V2:
>  - reword the patch description
>
>  net/ipv4/tcp.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index c82dc42f57c6..a4f418592314 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -4010,11 +4010,11 @@ int do_tcp_getsockopt(struct sock *sk, int level,
>         if (copy_from_sockptr(&len, optlen, sizeof(int)))
>                 return -EFAULT;
>
> -       len = min_t(unsigned int, len, sizeof(int));
> -
>         if (len < 0)
>                 return -EINVAL;
>
> +       len = min_t(unsigned int, len, sizeof(int));
> +
>         switch (optname) {
>         case TCP_MAXSEG:
>                 val = tp->mss_cache;
> --
> 2.39.2
>

[PATCH net-next v2 6/6] net/x25: fix incorrect parameter validation in the x25_getsockopt() function

[Gavrilov Ilia]

The 'len' variable can't be negative when assigned the result of
'min_t' because all 'min_t' parameters are cast to unsigned int,
and then the minimum one is chosen.

To fix the logic, check 'len' as read from 'optlen',
where the types of relevant variables are (signed) int.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Gavrilov Ilia <Ilia.Gavrilov@infotecs.ru>
---
 net/x25/af_x25.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c
index f7a7c7798c3b..d18d51412cc0 100644
--- a/net/x25/af_x25.c
+++ b/net/x25/af_x25.c
@@ -460,12 +460,12 @@ static int x25_getsockopt(struct socket *sock, int level, int optname,
 	if (get_user(len, optlen))
 		goto out;
 
-	len = min_t(unsigned int, len, sizeof(int));
-
 	rc = -EINVAL;
 	if (len < 0)
 		goto out;
 
+	len = min_t(unsigned int, len, sizeof(int));
+
 	rc = -EFAULT;
 	if (put_user(len, optlen))
 		goto out;
-- 
2.39.2

[PATCH net-next v2 4/6] l2tp: fix incorrect parameter validation in the pppol2tp_getsockopt() function

[Gavrilov Ilia]

The 'len' variable can't be negative when assigned the result of
'min_t' because all 'min_t' parameters are cast to unsigned int,
and then the minimum one is chosen.

To fix the logic, check 'len' as read from 'optlen',
where the types of relevant variables are (signed) int.

Fixes: 3557baabf280 ("[L2TP]: PPP over L2TP driver core")
Reviewed-by: Tom Parkin <tparkin@katalix.com>
Signed-off-by: Gavrilov Ilia <Ilia.Gavrilov@infotecs.ru>
---
V2:
 - reword the patch description
 net/l2tp/l2tp_ppp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c
index f011af6601c9..6146e4e67bbb 100644
--- a/net/l2tp/l2tp_ppp.c
+++ b/net/l2tp/l2tp_ppp.c
@@ -1356,11 +1356,11 @@ static int pppol2tp_getsockopt(struct socket *sock, int level, int optname,
 	if (get_user(len, optlen))
 		return -EFAULT;
 
-	len = min_t(unsigned int, len, sizeof(int));
-
 	if (len < 0)
 		return -EINVAL;
 
+	len = min_t(unsigned int, len, sizeof(int));
+
 	err = -ENOTCONN;
 	if (!sk->sk_user_data)
 		goto end;
-- 
2.39.2

[PATCH net-next v2 3/6] ipmr: fix incorrect parameter validation in the ip_mroute_getsockopt() function

[Gavrilov Ilia]

The 'olr' variable can't be negative when assigned the result of
'min_t' because all 'min_t' parameters are cast to unsigned int,
and then the minimum one is chosen.

To fix the logic, check 'olr' as read from 'optlen',
where the types of relevant variables are (signed) int.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Gavrilov Ilia <Ilia.Gavrilov@infotecs.ru>
---
V2:
 - reword the patch description
 net/ipv4/ipmr.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index 362229836510..b53c36c473a5 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -1603,9 +1603,11 @@ int ip_mroute_getsockopt(struct sock *sk, int optname, sockptr_t optval,
 
 	if (copy_from_sockptr(&olr, optlen, sizeof(int)))
 		return -EFAULT;
-	olr = min_t(unsigned int, olr, sizeof(int));
 	if (olr < 0)
 		return -EINVAL;
+
+	olr = min_t(unsigned int, olr, sizeof(int));
+
 	if (copy_to_sockptr(optlen, &olr, sizeof(int)))
 		return -EFAULT;
 	if (copy_to_sockptr(optval, &val, olr))
-- 
2.39.2

[PATCH net-next v2 5/6] net: kcm: fix incorrect parameter validation in the kcm_getsockopt) function

[Gavrilov Ilia]

The 'len' variable can't be negative when assigned the result of
'min_t' because all 'min_t' parameters are cast to unsigned int,
and then the minimum one is chosen.

To fix the logic, check 'len' as read from 'optlen',
where the types of relevant variables are (signed) int.

Fixes: ab7ac4eb9832 ("kcm: Kernel Connection Multiplexor module")
Signed-off-by: Gavrilov Ilia <Ilia.Gavrilov@infotecs.ru>
---
 net/kcm/kcmsock.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/kcm/kcmsock.c b/net/kcm/kcmsock.c
index 1184d40167b8..eda933c09792 100644
--- a/net/kcm/kcmsock.c
+++ b/net/kcm/kcmsock.c
@@ -1152,10 +1152,11 @@ static int kcm_getsockopt(struct socket *sock, int level, int optname,
 	if (get_user(len, optlen))
 		return -EFAULT;
 
-	len = min_t(unsigned int, len, sizeof(int));
 	if (len < 0)
 		return -EINVAL;
 
+	len = min_t(unsigned int, len, sizeof(int));
+
 	switch (optname) {
 	case KCM_RECV_DISABLE:
 		val = kcm->rx_disabled;
-- 
2.39.2

[PATCH net-next v2 2/6] udp: fix incorrect parameter validation in the udp_lib_getsockopt() function

[Gavrilov Ilia]

The 'len' variable can't be negative when assigned the result of
'min_t' because all 'min_t' parameters are cast to unsigned int,
and then the minimum one is chosen.

To fix the logic, check 'len' as read from 'optlen',
where the types of relevant variables are (signed) int.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reviewed-by: Willem de Bruijn <willemb@google.com>
Signed-off-by: Gavrilov Ilia <Ilia.Gavrilov@infotecs.ru>
---
V2:
 - reword the patch description
 net/ipv4/udp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index e474b201900f..17231c0f8830 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -2792,11 +2792,11 @@ int udp_lib_getsockopt(struct sock *sk, int level, int optname,
 	if (get_user(len, optlen))
 		return -EFAULT;
 
-	len = min_t(unsigned int, len, sizeof(int));
-
 	if (len < 0)
 		return -EINVAL;
 
+	len = min_t(unsigned int, len, sizeof(int));
+
 	switch (optname) {
 	case UDP_CORK:
 		val = udp_test_bit(CORK, sk);
-- 
2.39.2

Re: [PATCH net-next v2 0/6] fix incorrect parameter validation in the *_get_sockopt() functions

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by David S. Miller <davem@davemloft.net>:

On Thu, 7 Mar 2024 14:23:49 +0000 you wrote:
> This v2 series fix incorrent parameter validation in *_get_sockopt()
> functions in several places.
> 
> version 2 changes:
> - reword the patch description
> - add two patches for net/kcm and net/x25
> 
> [...]

Here is the summary with links:
  - [net-next,v2,1/6] tcp: fix incorrect parameter validation in the do_tcp_getsockopt() function
    https://git.kernel.org/netdev/net-next/c/716edc9706de
  - [net-next,v2,2/6] udp: fix incorrect parameter validation in the udp_lib_getsockopt() function
    https://git.kernel.org/netdev/net-next/c/4bb3ba7b74fc
  - [net-next,v2,3/6] ipmr: fix incorrect parameter validation in the ip_mroute_getsockopt() function
    https://git.kernel.org/netdev/net-next/c/5c3be3e0eb44
  - [net-next,v2,4/6] l2tp: fix incorrect parameter validation in the pppol2tp_getsockopt() function
    https://git.kernel.org/netdev/net-next/c/955e9876ba4e
  - [net-next,v2,5/6] net: kcm: fix incorrect parameter validation in the kcm_getsockopt) function
    https://git.kernel.org/netdev/net-next/c/3ed5f415133f
  - [net-next,v2,6/6] net/x25: fix incorrect parameter validation in the x25_getsockopt() function
    https://git.kernel.org/netdev/net-next/c/d6eb8de2015f

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html