Netdev Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] xfrm/compat: Fix general protection fault in xfrm_user_rcv_msg_compat()
@ 2021-07-12 13:40 YueHaibing
  2021-07-16  8:01 ` Steffen Klassert
  0 siblings, 1 reply; 3+ messages in thread
From: YueHaibing @ 2021-07-12 13:40 UTC (permalink / raw)
  To: steffen.klassert, herbert, davem, kuba, 0x7f454c46
  Cc: netdev, linux-kernel, YueHaibing

In xfrm_user_rcv_msg_compat() if maxtype is not zero and less than
XFRMA_MAX, nlmsg_parse_deprecated() do not initialize attrs array fully.
xfrm_xlate32() will access uninit 'attrs[i]' while iterating all attrs
array.

KASAN: probably user-memory-access in range [0x0000000041b58ab0-0x0000000041b58ab7]
CPU: 0 PID: 15799 Comm: syz-executor.2 Tainted: G        W         5.14.0-rc1-syzkaller #0
RIP: 0010:nla_type include/net/netlink.h:1130 [inline]
RIP: 0010:xfrm_xlate32_attr net/xfrm/xfrm_compat.c:410 [inline]
RIP: 0010:xfrm_xlate32 net/xfrm/xfrm_compat.c:532 [inline]
RIP: 0010:xfrm_user_rcv_msg_compat+0x5e5/0x1070 net/xfrm/xfrm_compat.c:577
[...]
Call Trace:
 xfrm_user_rcv_msg+0x556/0x8b0 net/xfrm/xfrm_user.c:2774
 netlink_rcv_skb+0x153/0x420 net/netlink/af_netlink.c:2504
 xfrm_netlink_rcv+0x6b/0x90 net/xfrm/xfrm_user.c:2824
 netlink_unicast_kernel net/netlink/af_netlink.c:1314 [inline]
 netlink_unicast+0x533/0x7d0 net/netlink/af_netlink.c:1340
 netlink_sendmsg+0x86d/0xdb0 net/netlink/af_netlink.c:1929
 sock_sendmsg_nosec net/socket.c:702 [inline]

Fixes: 5106f4a8acff ("xfrm/compat: Add 32=>64-bit messages translator")
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 net/xfrm/xfrm_compat.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/xfrm/xfrm_compat.c b/net/xfrm/xfrm_compat.c
index a20aec9d7393..4738660cadea 100644
--- a/net/xfrm/xfrm_compat.c
+++ b/net/xfrm/xfrm_compat.c
@@ -559,8 +559,8 @@ static struct nlmsghdr *xfrm_user_rcv_msg_compat(const struct nlmsghdr *h32,
 	    (h32->nlmsg_flags & NLM_F_DUMP))
 		return NULL;
 
-	err = nlmsg_parse_deprecated(h32, compat_msg_min[type], attrs,
-			maxtype ? : XFRMA_MAX, policy ? : compat_policy, extack);
+	err = nlmsg_parse_deprecated(h32, compat_msg_min[type], attrs, XFRMA_MAX,
+				     policy ? : compat_policy, extack);
 	if (err < 0)
 		return ERR_PTR(err);
 
-- 
2.20.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] xfrm/compat: Fix general protection fault in xfrm_user_rcv_msg_compat()
  2021-07-12 13:40 [PATCH] xfrm/compat: Fix general protection fault in xfrm_user_rcv_msg_compat() YueHaibing
@ 2021-07-16  8:01 ` Steffen Klassert
  2021-07-16 14:36   ` Dmitry Safonov
  0 siblings, 1 reply; 3+ messages in thread
From: Steffen Klassert @ 2021-07-16  8:01 UTC (permalink / raw)
  To: YueHaibing; +Cc: herbert, davem, kuba, 0x7f454c46, netdev, linux-kernel, dima

On Mon, Jul 12, 2021 at 09:40:02PM +0800, YueHaibing wrote:
> In xfrm_user_rcv_msg_compat() if maxtype is not zero and less than
> XFRMA_MAX, nlmsg_parse_deprecated() do not initialize attrs array fully.
> xfrm_xlate32() will access uninit 'attrs[i]' while iterating all attrs
> array.
> 
> KASAN: probably user-memory-access in range [0x0000000041b58ab0-0x0000000041b58ab7]
> CPU: 0 PID: 15799 Comm: syz-executor.2 Tainted: G        W         5.14.0-rc1-syzkaller #0
> RIP: 0010:nla_type include/net/netlink.h:1130 [inline]
> RIP: 0010:xfrm_xlate32_attr net/xfrm/xfrm_compat.c:410 [inline]
> RIP: 0010:xfrm_xlate32 net/xfrm/xfrm_compat.c:532 [inline]
> RIP: 0010:xfrm_user_rcv_msg_compat+0x5e5/0x1070 net/xfrm/xfrm_compat.c:577
> [...]
> Call Trace:
>  xfrm_user_rcv_msg+0x556/0x8b0 net/xfrm/xfrm_user.c:2774
>  netlink_rcv_skb+0x153/0x420 net/netlink/af_netlink.c:2504
>  xfrm_netlink_rcv+0x6b/0x90 net/xfrm/xfrm_user.c:2824
>  netlink_unicast_kernel net/netlink/af_netlink.c:1314 [inline]
>  netlink_unicast+0x533/0x7d0 net/netlink/af_netlink.c:1340
>  netlink_sendmsg+0x86d/0xdb0 net/netlink/af_netlink.c:1929
>  sock_sendmsg_nosec net/socket.c:702 [inline]
> 
> Fixes: 5106f4a8acff ("xfrm/compat: Add 32=>64-bit messages translator")
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> ---
>  net/xfrm/xfrm_compat.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/xfrm/xfrm_compat.c b/net/xfrm/xfrm_compat.c
> index a20aec9d7393..4738660cadea 100644
> --- a/net/xfrm/xfrm_compat.c
> +++ b/net/xfrm/xfrm_compat.c
> @@ -559,8 +559,8 @@ static struct nlmsghdr *xfrm_user_rcv_msg_compat(const struct nlmsghdr *h32,
>  	    (h32->nlmsg_flags & NLM_F_DUMP))
>  		return NULL;
>  
> -	err = nlmsg_parse_deprecated(h32, compat_msg_min[type], attrs,
> -			maxtype ? : XFRMA_MAX, policy ? : compat_policy, extack);
> +	err = nlmsg_parse_deprecated(h32, compat_msg_min[type], attrs, XFRMA_MAX,
> +				     policy ? : compat_policy, extack);

This removes the only usage of maxtype in that function. If we don't
need it, we should remove maxtype from the function parameters.

But looking closer at this, it seems that xfrm_xlate32() should
only iterate up to maxtype if set. Dimitry, any opinion on that?

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] xfrm/compat: Fix general protection fault in xfrm_user_rcv_msg_compat()
  2021-07-16  8:01 ` Steffen Klassert
@ 2021-07-16 14:36   ` Dmitry Safonov
  0 siblings, 0 replies; 3+ messages in thread
From: Dmitry Safonov @ 2021-07-16 14:36 UTC (permalink / raw)
  To: Steffen Klassert, YueHaibing
  Cc: herbert, davem, kuba, 0x7f454c46, netdev, linux-kernel

On 7/16/21 9:01 AM, Steffen Klassert wrote:
> On Mon, Jul 12, 2021 at 09:40:02PM +0800, YueHaibing wrote:
>> In xfrm_user_rcv_msg_compat() if maxtype is not zero and less than
>> XFRMA_MAX, nlmsg_parse_deprecated() do not initialize attrs array fully.
>> xfrm_xlate32() will access uninit 'attrs[i]' while iterating all attrs
>> array.
>>
>> KASAN: probably user-memory-access in range [0x0000000041b58ab0-0x0000000041b58ab7]
>> CPU: 0 PID: 15799 Comm: syz-executor.2 Tainted: G        W         5.14.0-rc1-syzkaller #0
>> RIP: 0010:nla_type include/net/netlink.h:1130 [inline]
>> RIP: 0010:xfrm_xlate32_attr net/xfrm/xfrm_compat.c:410 [inline]
>> RIP: 0010:xfrm_xlate32 net/xfrm/xfrm_compat.c:532 [inline]
>> RIP: 0010:xfrm_user_rcv_msg_compat+0x5e5/0x1070 net/xfrm/xfrm_compat.c:577
>> [...]
>> Call Trace:
>>  xfrm_user_rcv_msg+0x556/0x8b0 net/xfrm/xfrm_user.c:2774
>>  netlink_rcv_skb+0x153/0x420 net/netlink/af_netlink.c:2504
>>  xfrm_netlink_rcv+0x6b/0x90 net/xfrm/xfrm_user.c:2824
>>  netlink_unicast_kernel net/netlink/af_netlink.c:1314 [inline]
>>  netlink_unicast+0x533/0x7d0 net/netlink/af_netlink.c:1340
>>  netlink_sendmsg+0x86d/0xdb0 net/netlink/af_netlink.c:1929
>>  sock_sendmsg_nosec net/socket.c:702 [inline]
>>
>> Fixes: 5106f4a8acff ("xfrm/compat: Add 32=>64-bit messages translator")
>> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
>> ---
>>  net/xfrm/xfrm_compat.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/xfrm/xfrm_compat.c b/net/xfrm/xfrm_compat.c
>> index a20aec9d7393..4738660cadea 100644
>> --- a/net/xfrm/xfrm_compat.c
>> +++ b/net/xfrm/xfrm_compat.c
>> @@ -559,8 +559,8 @@ static struct nlmsghdr *xfrm_user_rcv_msg_compat(const struct nlmsghdr *h32,
>>  	    (h32->nlmsg_flags & NLM_F_DUMP))
>>  		return NULL;
>>  
>> -	err = nlmsg_parse_deprecated(h32, compat_msg_min[type], attrs,
>> -			maxtype ? : XFRMA_MAX, policy ? : compat_policy, extack);
>> +	err = nlmsg_parse_deprecated(h32, compat_msg_min[type], attrs, XFRMA_MAX,
>> +				     policy ? : compat_policy, extack);
> 
> This removes the only usage of maxtype in that function. If we don't
> need it, we should remove maxtype from the function parameters.
> 
> But looking closer at this, it seems that xfrm_xlate32() should
> only iterate up to maxtype if set. Dimitry, any opinion on that?
> 

Thanks for Cc. Yeah, I agree, it should pass maxtype to xfrm_xlate32().
More than that, it is XFRM_MSG_NEWSPDINFO, which have different possible
attributes: XFRMA_SPD_MAX vs XFRMA_MAX, so attribute translator
xfrm_xlate32_attr() should be corrected to translate these.

Let me fix this, thanks for the report!
I'll also add a selftest for this to xfrm selftest.

Thanks,
          Dmitry

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-07-16 14:36 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-12 13:40 [PATCH] xfrm/compat: Fix general protection fault in xfrm_user_rcv_msg_compat() YueHaibing
2021-07-16  8:01 ` Steffen Klassert
2021-07-16 14:36   ` Dmitry Safonov

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).