[syzbot] kernel BUG in warn_crc32c_csum_combine

16 views
Skip to first unread message

syzbot

unread,
Oct 23, 2022, 10:37:43 PM10/23/22
to da...@davemloft.net, edum...@google.com, her...@gondor.apana.org.au, ku...@kernel.org, linux-...@vger.kernel.org, net...@vger.kernel.org, pab...@redhat.com, steffen....@secunet.com, syzkall...@googlegroups.com
Hello,

syzbot found the following issue on:

HEAD commit: 4d48f589d294 Add linux-next specific files for 20221021
git tree: linux-next
console+strace: https://syzkaller.appspot.com/x/log.txt?x=1224e236880000
kernel config: https://syzkaller.appspot.com/x/.config?x=2c4b7d600a5739a6
dashboard link: https://syzkaller.appspot.com/bug?extid=1e9af9185d8850e2c2fa
compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=16f390f2880000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=171f9c8c880000

Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/0c86bd0b39a0/disk-4d48f589.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/074059d37f1f/vmlinux-4d48f589.xz
mounted in repro: https://storage.googleapis.com/syzbot-assets/4a30bce99f60/mount_1.gz

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+1e9af9...@syzkaller.appspotmail.com

------------[ cut here ]------------
kernel BUG at net/core/skbuff.c:120!
invalid opcode: 0000 [#1] PREEMPT SMP KASAN
CPU: 1 PID: 3637 Comm: syz-executor164 Not tainted 6.1.0-rc1-next-20221021-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 10/11/2022
RIP: 0010:skb_push.cold-0x2/0x24
Code: f8 4c 8b 4c 24 10 8b 4b 70 41 56 45 89 e8 4c 89 e2 41 57 48 89 ee 48 c7 c7 80 69 d4 8a ff 74 24 10 ff 74 24 20 e8 b2 77 c1 ff <0f> 0b e8 d4 d0 f1 f7 4c 8b 64 24 18 e8 ba 52 3e f8 48 c7 c1 e0 76
RSP: 0018:ffffc90003e7ee70 EFLAGS: 00010286
RAX: 0000000000000086 RBX: ffff888079c00280 RCX: 0000000000000000
RDX: ffff888020a3ba80 RSI: ffffffff81621a58 RDI: fffff520007cfdc0
RBP: ffffffff8ad47720 R08: 0000000000000086 R09: 0000000000000000
R10: 0000000080000000 R11: 0000000075626b73 R12: ffffffff883cc6c6
R13: 0000000000000048 R14: ffffffff8ad46940 R15: 00000000000000c0
FS: 00007f2b0a939700(0000) GS:ffff8880b9b00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f9f0b184060 CR3: 00000000755db000 CR4: 00000000003506e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
<TASK>
skb_over_panic net/core/skbuff.c:125 [inline]
warn_crc32c_csum_combine.cold+0x0/0x1d net/core/skbuff.c:2152
dump_esp_combs net/key/af_key.c:3009 [inline]
pfkey_send_acquire+0x1856/0x2520 net/key/af_key.c:3230
km_query+0xac/0x220 net/xfrm/xfrm_state.c:2248
xfrm_state_find+0x2bfe/0x4f10 net/xfrm/xfrm_state.c:1165
xfrm_tmpl_resolve_one net/xfrm/xfrm_policy.c:2392 [inline]
xfrm_tmpl_resolve+0x2f3/0xd40 net/xfrm/xfrm_policy.c:2437
xfrm_resolve_and_create_bundle+0x123/0x2580 net/xfrm/xfrm_policy.c:2730
xfrm_lookup_with_ifid+0x229/0x20f0 net/xfrm/xfrm_policy.c:3064
xfrm_lookup net/xfrm/xfrm_policy.c:3193 [inline]
xfrm_lookup_route+0x36/0x1e0 net/xfrm/xfrm_policy.c:3204
ip_route_output_flow+0x114/0x150 net/ipv4/route.c:2880
udp_sendmsg+0x1963/0x2740 net/ipv4/udp.c:1224
inet_sendmsg+0x99/0xe0 net/ipv4/af_inet.c:825
sock_sendmsg_nosec net/socket.c:714 [inline]
sock_sendmsg+0xcf/0x120 net/socket.c:734
____sys_sendmsg+0x334/0x8c0 net/socket.c:2482
___sys_sendmsg+0x110/0x1b0 net/socket.c:2536
__sys_sendmmsg+0x18b/0x460 net/socket.c:2622
__do_sys_sendmmsg net/socket.c:2651 [inline]
__se_sys_sendmmsg net/socket.c:2648 [inline]
__x64_sys_sendmmsg+0x99/0x100 net/socket.c:2648
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x63/0xcd
RIP: 0033:0x7f2b0a9adf89
Code: ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007f2b0a9392f8 EFLAGS: 00000246 ORIG_RAX: 0000000000000133
RAX: ffffffffffffffda RBX: 00007f2b0aa334d0 RCX: 00007f2b0a9adf89
RDX: 000000000800001d RSI: 0000000020007fc0 RDI: 0000000000000003
RBP: 00007f2b0aa002b8 R08: 0000000000000000 R09: 0000000000000000
R10: 000000a742250118 R11: 0000000000000246 R12: 00007f2b0aa000b8
R13: 00007f2b0aa001b8 R14: 0100000000000000 R15: 00007f2b0aa334d8
</TASK>
Modules linked in:
---[ end trace 0000000000000000 ]---
RIP: 0010:skb_push.cold-0x2/0x24
Code: f8 4c 8b 4c 24 10 8b 4b 70 41 56 45 89 e8 4c 89 e2 41 57 48 89 ee 48 c7 c7 80 69 d4 8a ff 74 24 10 ff 74 24 20 e8 b2 77 c1 ff <0f> 0b e8 d4 d0 f1 f7 4c 8b 64 24 18 e8 ba 52 3e f8 48 c7 c1 e0 76
RSP: 0018:ffffc90003e7ee70 EFLAGS: 00010286
RAX: 0000000000000086 RBX: ffff888079c00280 RCX: 0000000000000000
RDX: ffff888020a3ba80 RSI: ffffffff81621a58 RDI: fffff520007cfdc0
RBP: ffffffff8ad47720 R08: 0000000000000086 R09: 0000000000000000
R10: 0000000080000000 R11: 0000000075626b73 R12: ffffffff883cc6c6
R13: 0000000000000048 R14: ffffffff8ad46940 R15: 00000000000000c0
FS: 00007f2b0a939700(0000) GS:ffff8880b9b00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007fcc1415d300 CR3: 00000000755db000 CR4: 00000000003506e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400


---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzk...@googlegroups.com.

syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
syzbot can test patches for this issue, for details see:
https://goo.gl/tpsmEJ#testing-patches

Eric Dumazet

unread,
Oct 23, 2022, 10:49:57 PM10/23/22
to syzbot, da...@davemloft.net, her...@gondor.apana.org.au, ku...@kernel.org, linux-...@vger.kernel.org, net...@vger.kernel.org, pab...@redhat.com, steffen....@secunet.com, syzkall...@googlegroups.com
pfkey_send_acquire() allocates and skb, and then later this skb seems
to be too small to fit all dump info.

Maybe ->available status flips during the duration of the call ?

(So count_esp_combs() might return a value, but later dump_esp_combs()
needs more space)


Relevant patch suggests this could happen

commit ba953a9d89a00c078b85f4b190bc1dde66fe16b5
Author: Herbert Xu <her...@gondor.apana.org.au>
Date: Thu Aug 4 18:03:46 2022 +0800

af_key: Do not call xfrm_probe_algs in parallel

Herbert Xu

unread,
Oct 24, 2022, 1:01:57 AM10/24/22
to Eric Dumazet, syzbot, da...@davemloft.net, ku...@kernel.org, linux-...@vger.kernel.org, net...@vger.kernel.org, pab...@redhat.com, steffen....@secunet.com, syzkall...@googlegroups.com
On Sun, Oct 23, 2022 at 07:49:44PM -0700, Eric Dumazet wrote:
>
> pfkey_send_acquire() allocates and skb, and then later this skb seems
> to be too small to fit all dump info.
>
> Maybe ->available status flips during the duration of the call ?
>
> (So count_esp_combs() might return a value, but later dump_esp_combs()
> needs more space)

Thanks!

> Relevant patch suggests this could happen
>
> commit ba953a9d89a00c078b85f4b190bc1dde66fe16b5
> Author: Herbert Xu <her...@gondor.apana.org.au>
> Date: Thu Aug 4 18:03:46 2022 +0800
>
> af_key: Do not call xfrm_probe_algs in parallel

Yes this looks like the same issue just in a different spot.

Cheers,
--
Email: Herbert Xu <her...@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Herbert Xu

unread,
Oct 24, 2022, 1:10:57 AM10/24/22
to syzbot, da...@davemloft.net, edum...@google.com, ku...@kernel.org, linux-...@vger.kernel.org, net...@vger.kernel.org, pab...@redhat.com, steffen....@secunet.com, syzkall...@googlegroups.com
With name space support, it is possible for a pfkey_register to
occur in the middle of a send_acquire, thus changing the number
of supported algorithms.

This can be fixed by taking a mutex to make it single-threaded
again.

Reported-by: syzbot+1e9af9...@syzkaller.appspotmail.com
Fixes: 283bc9f35bbb ("xfrm: Namespacify xfrm state/policy locks")
Signed-off-by: Herbert Xu <her...@gondor.apana.org.au>

diff --git a/net/key/af_key.c b/net/key/af_key.c
index c85df5b958d2..4ceef96fef57 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -3160,6 +3160,7 @@ static int pfkey_send_acquire(struct xfrm_state *x, struct xfrm_tmpl *t, struct
(sockaddr_size * 2) +
sizeof(struct sadb_x_policy);

+ mutex_lock(&pfkey_mutex);
if (x->id.proto == IPPROTO_AH)
size += count_ah_combs(t);
else if (x->id.proto == IPPROTO_ESP)
@@ -3171,8 +3172,10 @@ static int pfkey_send_acquire(struct xfrm_state *x, struct xfrm_tmpl *t, struct
}

skb = alloc_skb(size + 16, GFP_ATOMIC);
- if (skb == NULL)
+ if (skb == NULL) {
+ mutex_unlock(&pfkey_mutex);
return -ENOMEM;
+ }

hdr = skb_put(skb, sizeof(struct sadb_msg));
hdr->sadb_msg_version = PF_KEY_V2;
@@ -3228,6 +3231,7 @@ static int pfkey_send_acquire(struct xfrm_state *x, struct xfrm_tmpl *t, struct
dump_ah_combs(skb, t);
else if (x->id.proto == IPPROTO_ESP)
dump_esp_combs(skb, t);
+ mutex_unlock(&pfkey_mutex);

/* security context */
if (xfrm_ctx) {

Eric Dumazet

unread,
Oct 24, 2022, 1:21:18 AM10/24/22
to Herbert Xu, syzbot, da...@davemloft.net, ku...@kernel.org, linux-...@vger.kernel.org, net...@vger.kernel.org, pab...@redhat.com, steffen....@secunet.com, syzkall...@googlegroups.com
On Sun, Oct 23, 2022 at 10:10 PM Herbert Xu <her...@gondor.apana.org.au> wrote:
>
> With name space support, it is possible for a pfkey_register to
> occur in the middle of a send_acquire, thus changing the number
> of supported algorithms.
>
> This can be fixed by taking a mutex to make it single-threaded
> again.
>
> Reported-by: syzbot+1e9af9...@syzkaller.appspotmail.com
> Fixes: 283bc9f35bbb ("xfrm: Namespacify xfrm state/policy locks")
> Signed-off-by: Herbert Xu <her...@gondor.apana.org.au>
>
> diff --git a/net/key/af_key.c b/net/key/af_key.c
> index c85df5b958d2..4ceef96fef57 100644
> --- a/net/key/af_key.c
> +++ b/net/key/af_key.c
> @@ -3160,6 +3160,7 @@ static int pfkey_send_acquire(struct xfrm_state *x, struct xfrm_tmpl *t, struct
> (sockaddr_size * 2) +
> sizeof(struct sadb_x_policy);
>
> + mutex_lock(&pfkey_mutex);
> if (x->id.proto == IPPROTO_AH)
> size += count_ah_combs(t);
> else if (x->id.proto == IPPROTO_ESP)
> @@ -3171,8 +3172,10 @@ static int pfkey_send_acquire(struct xfrm_state *x, struct xfrm_tmpl *t, struct
> }
>
> skb = alloc_skb(size + 16, GFP_ATOMIC);

Are you sure we can sleep in mutex_lock() ?

Use of GFP_ATOMIC would suggest otherwise :/

Herbert Xu

unread,
Oct 24, 2022, 2:06:36 AM10/24/22
to Eric Dumazet, syzbot, da...@davemloft.net, ku...@kernel.org, linux-...@vger.kernel.org, net...@vger.kernel.org, pab...@redhat.com, steffen....@secunet.com, syzkall...@googlegroups.com
On Sun, Oct 23, 2022 at 10:21:05PM -0700, Eric Dumazet wrote:
>
> Are you sure we can sleep in mutex_lock() ?
>
> Use of GFP_ATOMIC would suggest otherwise :/

Good point. Acquires are triggered from the network stack so
it may be in BH context.

---8<---
With name space support, it is possible for a pfkey_register to
occur in the middle of a send_acquire, thus changing the number
of supported algorithms.

This can be fixed by taking a lock to make it single-threaded
again. As this lock can be taken from both thread and softirq
contexts, we need to take the necessary precausions with disabling
BH and make it a spin lock.

Reported-by: syzbot+1e9af9...@syzkaller.appspotmail.com
Fixes: 283bc9f35bbb ("xfrm: Namespacify xfrm state/policy locks")
Signed-off-by: Herbert Xu <her...@gondor.apana.org.au>

diff --git a/net/key/af_key.c b/net/key/af_key.c
index c85df5b958d2..4e0d21e2045e 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -23,6 +23,7 @@
#include <linux/proc_fs.h>
#include <linux/init.h>
#include <linux/slab.h>
+#include <linux/spinlock.h>
#include <net/net_namespace.h>
#include <net/netns/generic.h>
#include <net/xfrm.h>
@@ -39,6 +40,7 @@ struct netns_pfkey {
atomic_t socks_nr;
};
static DEFINE_MUTEX(pfkey_mutex);
+static DEFINE_SPINLOCK(pfkey_alg_lock);

#define DUMMY_MARK 0
static const struct xfrm_mark dummy_mark = {0, 0};
@@ -1697,11 +1699,11 @@ static int pfkey_register(struct sock *sk, struct sk_buff *skb, const struct sad
pfk->registered |= (1<<hdr->sadb_msg_satype);
}

- mutex_lock(&pfkey_mutex);
+ spin_lock_bh(&pfkey_alg_lock);
xfrm_probe_algs();

supp_skb = compose_sadb_supported(hdr, GFP_KERNEL | __GFP_ZERO);
- mutex_unlock(&pfkey_mutex);
+ spin_unlock_bh(&pfkey_alg_lock);

if (!supp_skb) {
if (hdr->sadb_msg_satype != SADB_SATYPE_UNSPEC)
@@ -3160,6 +3162,7 @@ static int pfkey_send_acquire(struct xfrm_state *x, struct xfrm_tmpl *t, struct
(sockaddr_size * 2) +
sizeof(struct sadb_x_policy);

+ spin_lock_bh(&pfkey_alg_lock);
if (x->id.proto == IPPROTO_AH)
size += count_ah_combs(t);
else if (x->id.proto == IPPROTO_ESP)
@@ -3171,8 +3174,10 @@ static int pfkey_send_acquire(struct xfrm_state *x, struct xfrm_tmpl *t, struct
}

skb = alloc_skb(size + 16, GFP_ATOMIC);
- if (skb == NULL)
+ if (skb == NULL) {
+ spin_unlock_bh(&pfkey_alg_lock);
return -ENOMEM;
+ }

hdr = skb_put(skb, sizeof(struct sadb_msg));
hdr->sadb_msg_version = PF_KEY_V2;
@@ -3228,6 +3233,7 @@ static int pfkey_send_acquire(struct xfrm_state *x, struct xfrm_tmpl *t, struct
dump_ah_combs(skb, t);
else if (x->id.proto == IPPROTO_ESP)
dump_esp_combs(skb, t);
+ spin_unlock_bh(&pfkey_alg_lock);

Eric Dumazet

unread,
Oct 24, 2022, 2:31:12 AM10/24/22
to Herbert Xu, syzbot, da...@davemloft.net, ku...@kernel.org, linux-...@vger.kernel.org, net...@vger.kernel.org, pab...@redhat.com, steffen....@secunet.com, syzkall...@googlegroups.com
s/GFP_KERNEL/GFP_ATOMIC/

Sabrina Dubroca

unread,
Oct 24, 2022, 3:20:42 AM10/24/22
to Herbert Xu, Eric Dumazet, syzbot, da...@davemloft.net, ku...@kernel.org, linux-...@vger.kernel.org, net...@vger.kernel.org, pab...@redhat.com, steffen....@secunet.com, syzkall...@googlegroups.com
2022-10-24, 14:06:12 +0800, Herbert Xu wrote:
> @@ -1697,11 +1699,11 @@ static int pfkey_register(struct sock *sk, struct sk_buff *skb, const struct sad
> pfk->registered |= (1<<hdr->sadb_msg_satype);
> }
>
> - mutex_lock(&pfkey_mutex);
> + spin_lock_bh(&pfkey_alg_lock);
> xfrm_probe_algs();

I don't think we can do that:

void xfrm_probe_algs(void)
{
int i, status;

BUG_ON(in_softirq());


>
> supp_skb = compose_sadb_supported(hdr, GFP_KERNEL | __GFP_ZERO);
> - mutex_unlock(&pfkey_mutex);
> + spin_unlock_bh(&pfkey_alg_lock);
>
> if (!supp_skb) {
> if (hdr->sadb_msg_satype != SADB_SATYPE_UNSPEC)

--
Sabrina

Herbert Xu

unread,
Oct 25, 2022, 1:46:36 AM10/25/22
to Eric Dumazet, syzbot, da...@davemloft.net, ku...@kernel.org, linux-...@vger.kernel.org, net...@vger.kernel.org, pab...@redhat.com, steffen....@secunet.com, syzkall...@googlegroups.com
On Sun, Oct 23, 2022 at 11:31:00PM -0700, Eric Dumazet wrote:
>
> s/GFP_KERNEL/GFP_ATOMIC/

Thanks, I clearly wasn't thinking when I made this patch :)

Herbert Xu

unread,
Oct 25, 2022, 2:07:15 AM10/25/22
to Sabrina Dubroca, Eric Dumazet, syzbot, da...@davemloft.net, ku...@kernel.org, linux-...@vger.kernel.org, net...@vger.kernel.org, pab...@redhat.com, steffen....@secunet.com, syzkall...@googlegroups.com
On Mon, Oct 24, 2022 at 09:20:00AM +0200, Sabrina Dubroca wrote:
> 2022-10-24, 14:06:12 +0800, Herbert Xu wrote:
> > @@ -1697,11 +1699,11 @@ static int pfkey_register(struct sock *sk, struct sk_buff *skb, const struct sad
> > pfk->registered |= (1<<hdr->sadb_msg_satype);
> > }
> >
> > - mutex_lock(&pfkey_mutex);
> > + spin_lock_bh(&pfkey_alg_lock);
> > xfrm_probe_algs();
>
> I don't think we can do that:
>
> void xfrm_probe_algs(void)
> {
> int i, status;
>
> BUG_ON(in_softirq());

Indeed. I was also wrong in stating that this bug was created by
namespaces. This race has always existed since this code was first
added.

---8<---
The function pfkey_send_acquire may race with pfkey_register
(which could even be in a different name space). This may result
in a buffer overrun.

Allocating the maximum amount of memory that could be used prevents
this.

Reported-by: syzbot+1e9af9...@syzkaller.appspotmail.com
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Herbert Xu <her...@gondor.apana.org.au>

diff --git a/net/key/af_key.c b/net/key/af_key.c
index c85df5b958d2..213287814328 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -2905,7 +2905,7 @@ static int count_ah_combs(const struct xfrm_tmpl *t)
break;
if (!aalg->pfkey_supported)
continue;
- if (aalg_tmpl_set(t, aalg) && aalg->available)
+ if (aalg_tmpl_set(t, aalg))
sz += sizeof(struct sadb_comb);
}
return sz + sizeof(struct sadb_prop);
@@ -2923,7 +2923,7 @@ static int count_esp_combs(const struct xfrm_tmpl *t)
if (!ealg->pfkey_supported)
continue;

- if (!(ealg_tmpl_set(t, ealg) && ealg->available))
+ if (!(ealg_tmpl_set(t, ealg)))
continue;

for (k = 1; ; k++) {
@@ -2934,16 +2934,17 @@ static int count_esp_combs(const struct xfrm_tmpl *t)
if (!aalg->pfkey_supported)
continue;

- if (aalg_tmpl_set(t, aalg) && aalg->available)
+ if (aalg_tmpl_set(t, aalg))
sz += sizeof(struct sadb_comb);
}
}
return sz + sizeof(struct sadb_prop);
}

-static void dump_ah_combs(struct sk_buff *skb, const struct xfrm_tmpl *t)
+static int dump_ah_combs(struct sk_buff *skb, const struct xfrm_tmpl *t)
{
struct sadb_prop *p;
+ int sz = 0;
int i;

p = skb_put(skb, sizeof(struct sadb_prop));
@@ -2971,13 +2972,17 @@ static void dump_ah_combs(struct sk_buff *skb, const struct xfrm_tmpl *t)
c->sadb_comb_soft_addtime = 20*60*60;
c->sadb_comb_hard_usetime = 8*60*60;
c->sadb_comb_soft_usetime = 7*60*60;
+ sz += sizeof(*c);
}
}
+
+ return sz + sizeof(*p);
}

-static void dump_esp_combs(struct sk_buff *skb, const struct xfrm_tmpl *t)
+static int dump_esp_combs(struct sk_buff *skb, const struct xfrm_tmpl *t)
{
struct sadb_prop *p;
+ int sz = 0;
int i, k;

p = skb_put(skb, sizeof(struct sadb_prop));
@@ -3019,8 +3024,11 @@ static void dump_esp_combs(struct sk_buff *skb, const struct xfrm_tmpl *t)
c->sadb_comb_soft_addtime = 20*60*60;
c->sadb_comb_hard_usetime = 8*60*60;
c->sadb_comb_soft_usetime = 7*60*60;
+ sz += sizeof(*c);
}
}
+
+ return sz + sizeof(*p);
}

static int key_notify_policy_expire(struct xfrm_policy *xp, const struct km_event *c)
@@ -3150,6 +3158,7 @@ static int pfkey_send_acquire(struct xfrm_state *x, struct xfrm_tmpl *t, struct
struct sadb_x_sec_ctx *sec_ctx;
struct xfrm_sec_ctx *xfrm_ctx;
int ctx_size = 0;
+ int alg_size = 0;

sockaddr_size = pfkey_sockaddr_size(x->props.family);
if (!sockaddr_size)
@@ -3161,16 +3170,16 @@ static int pfkey_send_acquire(struct xfrm_state *x, struct xfrm_tmpl *t, struct
sizeof(struct sadb_x_policy);

if (x->id.proto == IPPROTO_AH)
- size += count_ah_combs(t);
+ alg_size = count_ah_combs(t);
else if (x->id.proto == IPPROTO_ESP)
- size += count_esp_combs(t);
+ alg_size = count_esp_combs(t);

if ((xfrm_ctx = x->security)) {
ctx_size = PFKEY_ALIGN8(xfrm_ctx->ctx_len);
size += sizeof(struct sadb_x_sec_ctx) + ctx_size;
}

- skb = alloc_skb(size + 16, GFP_ATOMIC);
+ skb = alloc_skb(size + alg_size + 16, GFP_ATOMIC);
if (skb == NULL)
return -ENOMEM;

@@ -3224,10 +3233,13 @@ static int pfkey_send_acquire(struct xfrm_state *x, struct xfrm_tmpl *t, struct
pol->sadb_x_policy_priority = xp->priority;

/* Set sadb_comb's. */
+ alg_size = 0;
if (x->id.proto == IPPROTO_AH)
- dump_ah_combs(skb, t);
+ alg_size = dump_ah_combs(skb, t);
else if (x->id.proto == IPPROTO_ESP)
- dump_esp_combs(skb, t);
+ alg_size = dump_esp_combs(skb, t);
+
+ hdr->sadb_msg_len += alg_size / 8;

Sabrina Dubroca

unread,
Oct 26, 2022, 9:39:06 AM10/26/22
to Herbert Xu, Eric Dumazet, syzbot, da...@davemloft.net, ku...@kernel.org, linux-...@vger.kernel.org, net...@vger.kernel.org, pab...@redhat.com, steffen....@secunet.com, syzkall...@googlegroups.com
2022-10-25, 14:06:48 +0800, Herbert Xu wrote:
> On Mon, Oct 24, 2022 at 09:20:00AM +0200, Sabrina Dubroca wrote:
> > 2022-10-24, 14:06:12 +0800, Herbert Xu wrote:
> > > @@ -1697,11 +1699,11 @@ static int pfkey_register(struct sock *sk, struct sk_buff *skb, const struct sad
> > > pfk->registered |= (1<<hdr->sadb_msg_satype);
> > > }
> > >
> > > - mutex_lock(&pfkey_mutex);
> > > + spin_lock_bh(&pfkey_alg_lock);
> > > xfrm_probe_algs();
> >
> > I don't think we can do that:
> >
> > void xfrm_probe_algs(void)
> > {
> > int i, status;
> >
> > BUG_ON(in_softirq());
>
> Indeed. I was also wrong in stating that this bug was created by
> namespaces. This race has always existed since this code was first
> added.
>
> ---8<---
> The function pfkey_send_acquire may race with pfkey_register
> (which could even be in a different name space). This may result
> in a buffer overrun.
>
> Allocating the maximum amount of memory that could be used prevents
> this.
>
> Reported-by: syzbot+1e9af9...@syzkaller.appspotmail.com
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Herbert Xu <her...@gondor.apana.org.au>

LGTM, thanks.

Reviewed-by: Sabrina Dubroca <s...@queasysnail.net>

--
Sabrina

Herbert Xu

unread,
Oct 26, 2022, 11:43:11 PM10/26/22
to Sabrina Dubroca, Eric Dumazet, syzbot, da...@davemloft.net, ku...@kernel.org, linux-...@vger.kernel.org, net...@vger.kernel.org, pab...@redhat.com, steffen....@secunet.com, syzkall...@googlegroups.com
On Wed, Oct 26, 2022 at 03:38:23PM +0200, Sabrina Dubroca wrote:
>
> LGTM, thanks.
>
> Reviewed-by: Sabrina Dubroca <s...@queasysnail.net>

Thanks for the review and comments!

Eric Dumazet

unread,
Oct 26, 2022, 11:46:10 PM10/26/22
to Herbert Xu, Sabrina Dubroca, syzbot, da...@davemloft.net, ku...@kernel.org, linux-...@vger.kernel.org, net...@vger.kernel.org, pab...@redhat.com, steffen....@secunet.com, syzkall...@googlegroups.com
On Wed, Oct 26, 2022 at 8:42 PM Herbert Xu <her...@gondor.apana.org.au> wrote:
>
> On Wed, Oct 26, 2022 at 03:38:23PM +0200, Sabrina Dubroca wrote:
> >
> > LGTM, thanks.
> >
> > Reviewed-by: Sabrina Dubroca <s...@queasysnail.net>
>
> Thanks for the review and comments!

SGTM, thanks for the fix.

Reviewed-by: Eric Dumazet <edum...@google.com>

Herbert Xu

unread,
Oct 26, 2022, 11:56:10 PM10/26/22
to Eric Dumazet, Sabrina Dubroca, syzbot, da...@davemloft.net, ku...@kernel.org, linux-...@vger.kernel.org, net...@vger.kernel.org, pab...@redhat.com, steffen....@secunet.com, syzkall...@googlegroups.com
On Wed, Oct 26, 2022 at 08:45:57PM -0700, Eric Dumazet wrote:
>
> SGTM, thanks for the fix.
>
> Reviewed-by: Eric Dumazet <edum...@google.com>

Thanks Eric!

Steffen Klassert

unread,
Oct 28, 2022, 7:26:26 AM10/28/22
to Eric Dumazet, Herbert Xu, Sabrina Dubroca, syzbot, da...@davemloft.net, ku...@kernel.org, linux-...@vger.kernel.org, net...@vger.kernel.org, pab...@redhat.com, syzkall...@googlegroups.com
Applied, thanks everyone!
Reply all
Reply to author
Forward
0 new messages