| Message ID | 20230228140126.2208-1-liaichun@huawei.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | af_key: Fix panic in dump_ah_combs() | expand |
On Tue, Feb 28, 2023 at 10:01:26PM +0800, Aichun Li wrote: > Because the exclamation mark (!) is removed, the return value calculated by > count_esp_combs is smaller than that before ealg->available is removed. And in > dump_esp_combs, the number of struct sadb_combs in skb_put is larger than > alloc, This result in a buffer overrun. > > [ 658.159619] ------------[ cut here ]------------ > [ 658.159629] kernel BUG at net/core/skbuff.c:110! > [ 658.159733] invalid opcode: 0000 [#1] SMP KASAN NOPTI > [ 658.160047] CPU: 14 PID: 107946 Comm: kernel_BUG_in_w Kdump: loaded Tainted: G I 5.10.0-60.18.0.50.x86_64 #1 Can you reproduce this on a mainline kernel? > diff --git a/net/key/af_key.c b/net/key/af_key.c > index 8bc7d3999..bf2859c37 100644 > --- a/net/key/af_key.c > +++ b/net/key/af_key.c > @@ -2927,7 +2927,7 @@ static int count_esp_combs(const struct xfrm_tmpl *t) > if (!ealg->pfkey_supported) > continue; > > - if (!(ealg_tmpl_set(t, ealg))) > + if (!(ealg_tmpl_set(t, ealg) && ealg->available)) > continue; This makes no sense. You are making the result of count_esp_combs smaller. How can that prevent a buffer overrun if it was too small to begin with? PS we could remove those brackets though, that would be a good clean-up patch. - if (!(ealg_tmpl_set(t, ealg))) + if (!ealg_tmpl_set(t, ealg)) Cheers,
diff --git a/net/key/af_key.c b/net/key/af_key.c index 8bc7d3999..bf2859c37 100644 --- a/net/key/af_key.c +++ b/net/key/af_key.c @@ -2927,7 +2927,7 @@ static int count_esp_combs(const struct xfrm_tmpl *t) if (!ealg->pfkey_supported) continue; - if (!(ealg_tmpl_set(t, ealg))) + if (!(ealg_tmpl_set(t, ealg) && ealg->available)) continue; for (k = 1; ; k++) {
Because the exclamation mark (!) is removed, the return value calculated by count_esp_combs is smaller than that before ealg->available is removed. And in dump_esp_combs, the number of struct sadb_combs in skb_put is larger than alloc, This result in a buffer overrun. [ 658.159619] ------------[ cut here ]------------ [ 658.159629] kernel BUG at net/core/skbuff.c:110! [ 658.159733] invalid opcode: 0000 [#1] SMP KASAN NOPTI [ 658.160047] CPU: 14 PID: 107946 Comm: kernel_BUG_in_w Kdump: loaded Tainted: G I 5.10.0-60.18.0.50.x86_64 #1 [ 658.160179] Hardware name: Huawei 2288H V5/BC11SPSCB0, BIOS 8.20 10/20/2021 [ 658.160308] RIP: 0010:skb_panic+0xda/0xec [ 658.160416] Code: e8 48 c7 c7 00 a3 c2 a3 41 56 48 8b 74 24 18 4d 89 f9 56 48 8b 54 24 18 48 89 ee 52 48 8b 44 24 18 4c 89 e2 50 e8 6a 27 f6 ff <0f> 0b 48 c7 c7 a0 63 ca a4 48 83 c4 20 e8 d3 f6 f9 ff e8 2e 13 ea [ 658.160567] RSP: 0018:ffff88bba055eec8 EFLAGS: 00010282 [ 658.160675] RAX: 0000000000000089 RBX: ffff888265bffac0 RCX: 0000000000000000 [ 658.160795] RDX: ffff88b5af172200 RSI: 0000000000000008 RDI: ffffed17740abdcb [ 658.160919] RBP: ffffffffa3c2a820 R08: 0000000000000089 R09: ffff88a37a340727 [ 658.161039] R10: ffffed146f4680e4 R11: 0000000000000001 R12: ffffffffc1708e4d [ 658.161160] R13: 0000000000000048 R14: ffffffffa3c2a2c0 R15: ffff8883f0d73000 [ 658.161281] FS: 00007fd756c30640(0000) GS:ffff88a37a300000(0000) knlGS:0000000000000000 [ 658.161403] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 658.161509] CR2: 00007f40ed3a56e4 CR3: 000000384d578001 CR4: 00000000007706e0 [ 658.161629] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 658.161749] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 [ 658.161867] PKRU: 55555554 [ 658.161966] Call Trace: [ 658.162181] skb_put.cold+0x24/0x24 [ 658.162288] dump_esp_combs+0x1ad/0x330 [af_key] [ 658.162396] pfkey_send_acquire+0x66e/0x6b0 [af_key] [ 658.162508] xfrm_state_find+0x137b/0x1c00 [ 658.163161] xfrm_tmpl_resolve_one+0x171/0x640 [ 658.164651] xfrm_tmpl_resolve+0x14d/0x2e0 [ 658.165077] xfrm_resolve_and_create_bundle+0xb8/0x250 Fixes: 7f57f8165cb6 ("af_key: Fix send_acquire race with pfkey_register") Signed-off-by: Aichun Li <liaichun@huawei.com> Signed-off-by: zhongxuan <zhongxuan2@huawei.com> --- net/key/af_key.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)