net/xfrm: stack out-of-bounds in xfrm_flowi_sport
Hello,
The following program triggers stack out-of-bounds in xfrm_flowi_sport:
BUG: KASAN: stack-out-of-bounds in xfrm_flowi_sport
include/net/xfrm.h:862 [inline] at addr 8800677df796
BUG: KASAN: stack-out-of-bounds in __xfrm6_selector_match
net/xfrm/xfrm_policy.c:89 [inline] at addr 8800677df796
BUG: KASAN: stack-out-of-bounds in xfrm_selector_match+0xc94/0xe40
net/xfrm/xfrm_policy.c:101 at addr 8800677df796
Read of size 2 by task a.out/3338
page:ea00016a38c8 count:0 mapcount:0 mapping: (null) index:0x0
flags: 0x100()
raw: 0100
raw: dead0200
page dumped because: kasan: bad access detected
CPU: 0 PID: 3338 Comm: a.out Tainted: GB 4.10.0-rc8+ #218
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
Call Trace:
__asan_report_load2_noabort+0x29/0x30 mm/kasan/report.c:329
xfrm_flowi_sport include/net/xfrm.h:862 [inline]
__xfrm6_selector_match net/xfrm/xfrm_policy.c:89 [inline]
xfrm_selector_match+0xc94/0xe40 net/xfrm/xfrm_policy.c:101
xfrm_sk_policy_lookup+0x1bb/0x540 net/xfrm/xfrm_policy.c:1259
xfrm_lookup+0x215/0x10f0 net/xfrm/xfrm_policy.c:2256
xfrm_lookup_route+0x39/0x1a0 net/xfrm/xfrm_policy.c:2390
ip_route_output_flow+0x7f/0xa0 net/ipv4/route.c:2447
udp_sendmsg+0x162b/0x2b80 net/ipv4/udp.c:1027
udpv6_sendmsg+0x10a9/0x3170 net/ipv6/udp.c:1065
inet_sendmsg+0x164/0x5b0 net/ipv4/af_inet.c:744
sock_sendmsg_nosec net/socket.c:635 [inline]
sock_sendmsg+0xca/0x110 net/socket.c:645
SYSC_sendto+0x660/0x810 net/socket.c:1687
SyS_sendto+0x40/0x50 net/socket.c:1655
entry_SYSCALL_64_fastpath+0x1f/0xc2
RIP: 0033:0x436ed3
RSP: 002b:7fff6d081748 EFLAGS: 0246 ORIG_RAX: 002c
RAX: ffda RBX: 00401860 RCX: 00436ed3
RDX: 008f RSI: 20003f71 RDI: 0003
RBP: R08: 20003000 R09: 0010
R10: 00040084 R11: 0246 R12: 004002b0
R13: 00401860 R14: 004018f0 R15:
Memory state around the buggy address:
8800677df680: f1 00 f2 f2 f2 f2 f2 f2 f2 f8 f2 f2 f2 f2 f2 f2
8800677df700: f2 00 00 00 00 f2 f2 f2 f2 00 00 00 00 00 00 00
>8800677df780: f2 f2 f2 f2 f2 00 00 00 00 00 00 00 00 00 f2 f2
^
8800677df800: f2 f3 f3 f3 f3 00 00 00 00 00 00 00 00 00 00 00
8800677df880: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
==
// autogenerated by syzkaller (http://github.com/google/syzkaller)
#define _GNU_SOURCE
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
int main()
{
mmap((void*)0x2000ul, 0xfff000ul, 0x3ul, 0x32ul, -1, 0);
int sock = socket(AF_INET6, SOCK_DGRAM, IPPROTO_IP);
(*(uint64_t*)0x2098 = (uint64_t)0x20b5);
(*(uint32_t*)0x20a0 = (uint32_t)0x1);
(*(uint64_t*)0x2000 = (uint64_t)0x20005000);
(*(uint64_t*)0x2008 = (uint64_t)0x20002fdc);
(*(uint64_t*)0x2010 = (uint64_t)0x20001ff8);
(*(uint64_t*)0x2018 = (uint64_t)0x20004ff0);
(*(uint32_t*)0x2020 = (uint32_t)0x1);
(*(uint32_t*)0x2024 = (uint32_t)0x0);
(*(uint32_t*)0x2028 = (uint32_t)0x2);
(*(uint32_t*)0x202c = (uint32_t)0x0);
(*(uint32_t*)0x2030 = (uint32_t)0x0);
(*(uint32_t*)0x2034 = (uint32_t)0x0);
(*(uint32_t*)0x2038 = (uint32_t)0x0);
(*(uint32_t*)0x203c = (uint32_t)0x0);
setsockopt(sock, 0x29ul, 0x23ul, (void*)0x2000ul, 0x264ul);
(*(uint16_t*)0x20003000 = (uint16_t)0x2);
(*(uint16_t*)0x20003002 = (uint16_t)0x214e);
(*(uint32_t*)0x20003004 = (uint32_t)0x0);
(*(uint8_t*)0x20003008 = (uint8_t)0x0);
(*(uint8_t*)0x20003009 = (uint8_t)0x0);
(*(uint8_t*)0x2000300a = (uint8_t)0x0);
(*(uint8_t*)0x2000300b = (uint8_t)0x0);
(*(uint8_t*)0x2000300c = (uint8_t)0x0);
(*(uint8_t*)0x2000300d = (uint8_t)0x0);
(*(uint8_t*)0x2000300e = (uint8_t)0x0);
(*(uint8_t*)0x2000300f = (uint8_t)0x0);
sendto(sock, (void*)0x20003f71ul, 0x8ful, 0x40084ul,
(void*)0x20003000ul, 0x10ul);
return 0;
}
On commit 7089db84e356562f8ba737c29e472cc42d530dbc.
struct flowi4 fl4_stack allocated on stack in udp_sendmsg is being
casted to larger struct flowi and then accessed.
Re: net/xfrm: stack out-of-bounds in xfrm_flowi_sport
Hi Dmitry. On Tue, Feb 28, 2017 at 02:39:17PM +0100, Dmitry Vyukov wrote: > On Wed, Feb 15, 2017 at 7:26 AM, Steffen Klassert > wrote: > > On Tue, Feb 14, 2017 at 10:16:44AM +0100, Dmitry Vyukov wrote: > >> > >> I've run the repro with you patch and don't see the bug any more: > >> > >> Tested-by: Dmitry Vyukov > > > > I've applied this to the ipsec tree now. > > > > Thanks for testing! > > > Hi Steffen, > > Are you going to push this to Linus in this release? Still happens on > upstream branch. I'll do a pull request for the ipsec tree this week, should go the way upstream then.
Re: net/xfrm: stack out-of-bounds in xfrm_flowi_sport
On Wed, Feb 15, 2017 at 7:26 AM, Steffen Klassert wrote: > On Tue, Feb 14, 2017 at 10:16:44AM +0100, Dmitry Vyukov wrote: >> >> I've run the repro with you patch and don't see the bug any more: >> >> Tested-by: Dmitry Vyukov > > I've applied this to the ipsec tree now. > > Thanks for testing! Hi Steffen, Are you going to push this to Linus in this release? Still happens on upstream branch.
Re: net/xfrm: stack out-of-bounds in xfrm_flowi_sport
On Mon, Feb 13, 2017 at 03:46:56PM +0100, Dmitry Vyukov wrote:
>
> On commit 7089db84e356562f8ba737c29e472cc42d530dbc.
>
>
> struct flowi4 fl4_stack allocated on stack in udp_sendmsg is being
> casted to larger struct flowi and then accessed.
Looks like the problem is when using IPv4-mapped IPv6 addresses.
Does the patch below help?
Subject: [PATCH RFC ipsec] xfrm: Don't use sk_family for socket policy lookups
On IPv4-mapped IPv6 addresses sk_family is AF_INET6,
but the flow informations are created based on AF_INET.
So the routing set up 'struct flowi4' but we try to
access 'struct flowi6' what leads to an out of bounds
access. Fix this by using the family we get with the
dst_entry, like we do it for the standard policy lookup.
Reported-by: Dmitry Vyukov
Signed-off-by: Steffen Klassert
---
net/xfrm/xfrm_policy.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index b5e665b..4891b7b 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -1216,7 +1216,7 @@ static inline int policy_to_flow_dir(int dir)
}
static struct xfrm_policy *xfrm_sk_policy_lookup(const struct sock *sk, int
dir,
-const struct flowi *fl)
+const struct flowi *fl, u16
family)
{
struct xfrm_policy *pol;
struct net *net = sock_net(sk);
@@ -1225,8 +1225,7 @@ static struct xfrm_policy *xfrm_sk_policy_lookup(const
struct sock *sk, int dir,
read_lock_bh(&net->xfrm.xfrm_policy_lock);
pol = rcu_dereference(sk->sk_policy[dir]);
if (pol != NULL) {
- bool match = xfrm_selector_match(&pol->selector, fl,
-sk->sk_family);
+ bool match = xfrm_selector_match(&pol->selector, fl, family);
int err = 0;
if (match) {
@@ -2221,7 +2220,7 @@ struct dst_entry *xfrm_lookup(struct net *net, struct
dst_entry *dst_orig,
sk = sk_const_to_full_sk(sk);
if (sk && sk->sk_policy[XFRM_POLICY_OUT]) {
num_pols = 1;
- pols[0] = xfrm_sk_policy_lookup(sk, XFRM_POLICY_OUT, fl);
+ pols[0] = xfrm_sk_policy_lookup(sk, XFRM_POLICY_OUT, fl,
family);
err = xfrm_expand_policies(fl, family, pols,
&num_pols, &num_xfrms);
if (err < 0)
@@ -2500,7 +2499,7 @@ int __xfrm_policy_check(struct sock *sk, int dir, struct
sk_buff *skb,
pol = NULL;
sk = sk_to_full_sk(sk);
if (sk && sk->sk_policy[dir]) {
- pol = xfrm_sk_policy_lookup(sk, dir, &fl);
+ pol = xfrm_sk_policy_lookup(sk, dir, &fl, family);
if (IS_ERR(pol)) {
XFRM_INC_STATS(net, LINUX_MIB_XFRMINPOLERROR);
return 0;
--
1.9.1
Re: net/xfrm: stack out-of-bounds in xfrm_flowi_sport
On Tue, Feb 14, 2017 at 8:08 AM, Steffen Klassert
wrote:
> On Mon, Feb 13, 2017 at 03:46:56PM +0100, Dmitry Vyukov wrote:
>>
>> On commit 7089db84e356562f8ba737c29e472cc42d530dbc.
>>
>>
>> struct flowi4 fl4_stack allocated on stack in udp_sendmsg is being
>> casted to larger struct flowi and then accessed.
>
> Looks like the problem is when using IPv4-mapped IPv6 addresses.
>
> Does the patch below help?
Steffen, can you please run the reproducer I provided?
I specifically spent time to supply you with a simple, reliable
reproducer. I am not even saying about adding a test case for the bug.
Kernel development practices seem to encourage developers to not
bother with tests. But at least testing a patch that you are sending
looks like a reasonable thing to do.
Thanks
> Subject: [PATCH RFC ipsec] xfrm: Don't use sk_family for socket policy lookups
>
> On IPv4-mapped IPv6 addresses sk_family is AF_INET6,
> but the flow informations are created based on AF_INET.
> So the routing set up 'struct flowi4' but we try to
> access 'struct flowi6' what leads to an out of bounds
> access. Fix this by using the family we get with the
> dst_entry, like we do it for the standard policy lookup.
>
> Reported-by: Dmitry Vyukov
> Signed-off-by: Steffen Klassert
> ---
> net/xfrm/xfrm_policy.c | 9 -
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
> index b5e665b..4891b7b 100644
> --- a/net/xfrm/xfrm_policy.c
> +++ b/net/xfrm/xfrm_policy.c
> @@ -1216,7 +1216,7 @@ static inline int policy_to_flow_dir(int dir)
> }
>
> static struct xfrm_policy *xfrm_sk_policy_lookup(const struct sock *sk, int
> dir,
> -const struct flowi *fl)
> +const struct flowi *fl, u16
> family)
> {
> struct xfrm_policy *pol;
> struct net *net = sock_net(sk);
> @@ -1225,8 +1225,7 @@ static struct xfrm_policy *xfrm_sk_policy_lookup(const
> struct sock *sk, int dir,
> read_lock_bh(&net->xfrm.xfrm_policy_lock);
> pol = rcu_dereference(sk->sk_policy[dir]);
> if (pol != NULL) {
> - bool match = xfrm_selector_match(&pol->selector, fl,
> -sk->sk_family);
> + bool match = xfrm_selector_match(&pol->selector, fl, family);
> int err = 0;
>
> if (match) {
> @@ -2221,7 +2220,7 @@ struct dst_entry *xfrm_lookup(struct net *net, struct
> dst_entry *dst_orig,
> sk = sk_const_to_full_sk(sk);
> if (sk && sk->sk_policy[XFRM_POLICY_OUT]) {
> num_pols = 1;
> - pols[0] = xfrm_sk_policy_lookup(sk, XFRM_POLICY_OUT, fl);
> + pols[0] = xfrm_sk_policy_lookup(sk, XFRM_POLICY_OUT, fl,
> family);
> err = xfrm_expand_policies(fl, family, pols,
>&num_pols, &num_xfrms);
> if (err < 0)
> @@ -2500,7 +2499,7 @@ int __xfrm_policy_check(struct sock *sk, int dir,
> struct sk_buff *skb,
> pol = NULL;
> sk = sk_to_full_sk(sk);
> if (sk && sk->sk_policy[dir]) {
> - pol = xfrm_sk_policy_lookup(sk, dir, &fl);
> + pol = xfrm_sk_policy_lookup(sk, dir, &fl, family);
> if (IS_ERR(pol)) {
> XFRM_INC_STATS(net, LINUX_MIB_XFRMINPOLERROR);
> return 0;
> --
> 1.9.1
>
Re: net/xfrm: stack out-of-bounds in xfrm_flowi_sport
On Tue, Feb 14, 2017 at 09:41:35AM +0100, Dmitry Vyukov wrote: > On Tue, Feb 14, 2017 at 8:08 AM, Steffen Klassert > wrote: > > On Mon, Feb 13, 2017 at 03:46:56PM +0100, Dmitry Vyukov wrote: > >> > >> On commit 7089db84e356562f8ba737c29e472cc42d530dbc. > >> > >> > >> struct flowi4 fl4_stack allocated on stack in udp_sendmsg is being > >> casted to larger struct flowi and then accessed. > > > > Looks like the problem is when using IPv4-mapped IPv6 addresses. > > > > Does the patch below help? > > > Steffen, can you please run the reproducer I provided? > I specifically spent time to supply you with a simple, reliable > reproducer. I am not even saying about adding a test case for the bug. > Kernel development practices seem to encourage developers to not > bother with tests. But at least testing a patch that you are sending > looks like a reasonable thing to do. I tested this with my socket policy testcases of course. I dont have a IPv4-mapped IPv6 addresses testcase and changing userspace in my test setup means to rebuild the system iso image. Asking for a test is not so uncommon. You have the testcase, why not running it again?
Re: net/xfrm: stack out-of-bounds in xfrm_flowi_sport
On Tue, Feb 14, 2017 at 10:08 AM, Steffen Klassert wrote: > On Tue, Feb 14, 2017 at 09:41:35AM +0100, Dmitry Vyukov wrote: >> On Tue, Feb 14, 2017 at 8:08 AM, Steffen Klassert >> wrote: >> > On Mon, Feb 13, 2017 at 03:46:56PM +0100, Dmitry Vyukov wrote: >> >> >> >> On commit 7089db84e356562f8ba737c29e472cc42d530dbc. >> >> >> >> >> >> struct flowi4 fl4_stack allocated on stack in udp_sendmsg is being >> >> casted to larger struct flowi and then accessed. >> > >> > Looks like the problem is when using IPv4-mapped IPv6 addresses. >> > >> > Does the patch below help? >> >> >> Steffen, can you please run the reproducer I provided? >> I specifically spent time to supply you with a simple, reliable >> reproducer. I am not even saying about adding a test case for the bug. >> Kernel development practices seem to encourage developers to not >> bother with tests. But at least testing a patch that you are sending >> looks like a reasonable thing to do. > > I tested this with my socket policy testcases of course. > I dont have a IPv4-mapped IPv6 addresses testcase and > changing userspace in my test setup means to rebuild > the system iso image. > > Asking for a test is not so uncommon. You have the > testcase, why not running it again? Because there are too many kernel subsystems and bugs. I need more than 24h per day to reports and retest them. I've run the repro with you patch and don't see the bug any more: Tested-by: Dmitry Vyukov
Re: net/xfrm: stack out-of-bounds in xfrm_flowi_sport
On Tue, Feb 14, 2017 at 10:16:44AM +0100, Dmitry Vyukov wrote: > > I've run the repro with you patch and don't see the bug any more: > > Tested-by: Dmitry Vyukov I've applied this to the ipsec tree now. Thanks for testing!
