lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c0b30b6d-4647-2087-5684-0033c2ff6539@mellanox.com>
Date:   Wed, 27 Feb 2019 08:14:24 +0000
From:   Roi Dayan <roid@...lanox.com>
To:     Vlad Buslov <vladbu@...lanox.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "dcaratti@...hat.com" <dcaratti@...hat.com>
CC:     "jhs@...atatu.com" <jhs@...atatu.com>,
        "xiyou.wangcong@...il.com" <xiyou.wangcong@...il.com>,
        "jiri@...nulli.us" <jiri@...nulli.us>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "wenxu@...oud.cn" <wenxu@...oud.cn>
Subject: Re: [PATCH net-next v2] net: sched: act_tunnel_key: fix metadata
 handling



On 25/02/2019 17:30, Vlad Buslov wrote:
> Tunnel key action params->tcft_enc_metadata is only set when action is
> TCA_TUNNEL_KEY_ACT_SET. However, metadata pointer is incorrectly
> dereferenced during tunnel key init and release without verifying that
> action is if correct type, which causes NULL pointer dereference. Metadata
> tunnel dst_cache is also leaked on action overwrite.
> 
> Fix metadata handling:
> - Verify that metadata pointer is not NULL before dereferencing it in
>   tunnel_key_init error handling code.
> - Move dst_cache destroy code into tunnel_key_release_params() function
>   that is called in both action overwrite and release cases (fixes resource
>   leak) and verifies that actions has correct type before dereferencing
>   metadata pointer (fixes NULL pointer dereference).
> 
> Oops with KASAN enabled during tdc tests execution:
> 
> [  261.080482] ==================================================================
> [  261.088049] BUG: KASAN: null-ptr-deref in dst_cache_destroy+0x21/0xa0
> [  261.094613] Read of size 8 at addr 00000000000000b0 by task tc/2976
> [  261.102524] CPU: 14 PID: 2976 Comm: tc Not tainted 5.0.0-rc7+ #157
> [  261.108844] Hardware name: Supermicro SYS-2028TP-DECR/X10DRT-P, BIOS 2.0b 03/30/2017
> [  261.116726] Call Trace:
> [  261.119234]  dump_stack+0x9a/0xeb
> [  261.122625]  ? dst_cache_destroy+0x21/0xa0
> [  261.126818]  ? dst_cache_destroy+0x21/0xa0
> [  261.131004]  kasan_report+0x176/0x192
> [  261.134752]  ? idr_get_next+0xd0/0x120
> [  261.138578]  ? dst_cache_destroy+0x21/0xa0
> [  261.142768]  dst_cache_destroy+0x21/0xa0
> [  261.146799]  tunnel_key_release+0x3a/0x50 [act_tunnel_key]
> [  261.152392]  tcf_action_cleanup+0x2c/0xc0
> [  261.156490]  tcf_generic_walker+0x4c2/0x5c0
> [  261.160794]  ? tcf_action_dump_1+0x390/0x390
> [  261.165163]  ? tunnel_key_walker+0x5/0x1a0 [act_tunnel_key]
> [  261.170865]  ? tunnel_key_walker+0xe9/0x1a0 [act_tunnel_key]
> [  261.176641]  tca_action_gd+0x600/0xa40
> [  261.180482]  ? tca_get_fill.constprop.17+0x200/0x200
> [  261.185548]  ? __lock_acquire+0x588/0x1d20
> [  261.189741]  ? __lock_acquire+0x588/0x1d20
> [  261.193922]  ? mark_held_locks+0x90/0x90
> [  261.197944]  ? mark_held_locks+0x90/0x90
> [  261.202018]  ? __nla_parse+0xfe/0x190
> [  261.205774]  tc_ctl_action+0x218/0x230
> [  261.209614]  ? tcf_action_add+0x230/0x230
> [  261.213726]  rtnetlink_rcv_msg+0x3a5/0x600
> [  261.217910]  ? lock_downgrade+0x2d0/0x2d0
> [  261.222006]  ? validate_linkmsg+0x400/0x400
> [  261.226278]  ? find_held_lock+0x6d/0xd0
> [  261.230200]  ? match_held_lock+0x1b/0x210
> [  261.234296]  ? validate_linkmsg+0x400/0x400
> [  261.238567]  netlink_rcv_skb+0xc7/0x1f0
> [  261.242489]  ? netlink_ack+0x470/0x470
> [  261.246319]  ? netlink_deliver_tap+0x1f3/0x5a0
> [  261.250874]  netlink_unicast+0x2ae/0x350
> [  261.254884]  ? netlink_attachskb+0x340/0x340
> [  261.261647]  ? _copy_from_iter_full+0xdd/0x380
> [  261.268576]  ? __virt_addr_valid+0xb6/0xf0
> [  261.275227]  ? __check_object_size+0x159/0x240
> [  261.282184]  netlink_sendmsg+0x4d3/0x630
> [  261.288572]  ? netlink_unicast+0x350/0x350
> [  261.295132]  ? netlink_unicast+0x350/0x350
> [  261.301608]  sock_sendmsg+0x6d/0x80
> [  261.307467]  ___sys_sendmsg+0x48e/0x540
> [  261.313633]  ? copy_msghdr_from_user+0x210/0x210
> [  261.320545]  ? save_stack+0x89/0xb0
> [  261.326289]  ? __lock_acquire+0x588/0x1d20
> [  261.332605]  ? entry_SYSCALL_64_after_hwframe+0x49/0xbe
> [  261.340063]  ? mark_held_locks+0x90/0x90
> [  261.346162]  ? do_filp_open+0x138/0x1d0
> [  261.352108]  ? may_open_dev+0x50/0x50
> [  261.357897]  ? match_held_lock+0x1b/0x210
> [  261.364016]  ? __fget_light+0xa6/0xe0
> [  261.369840]  ? __sys_sendmsg+0xd2/0x150
> [  261.375814]  __sys_sendmsg+0xd2/0x150
> [  261.381610]  ? __ia32_sys_shutdown+0x30/0x30
> [  261.388026]  ? lock_downgrade+0x2d0/0x2d0
> [  261.394182]  ? mark_held_locks+0x1c/0x90
> [  261.400230]  ? do_syscall_64+0x1e/0x280
> [  261.406172]  do_syscall_64+0x78/0x280
> [  261.411932]  entry_SYSCALL_64_after_hwframe+0x49/0xbe
> [  261.419103] RIP: 0033:0x7f28e91a8b87
> [  261.424791] Code: 64 89 02 48 c7 c0 ff ff ff ff eb b9 0f 1f 80 00 00 00 00 8b 05 6a 2b 2c 00 48 63 d2 48 63 ff 85 c0 75 18 b8 2e 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 59 f3 c3 0f 1f 80 00 00 00 00 53 48 89 f3 48
> [  261.448226] RSP: 002b:00007ffdc5c4e2d8 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
> [  261.458183] RAX: ffffffffffffffda RBX: 000000005c73c202 RCX: 00007f28e91a8b87
> [  261.467728] RDX: 0000000000000000 RSI: 00007ffdc5c4e340 RDI: 0000000000000003
> [  261.477342] RBP: 0000000000000000 R08: 0000000000000001 R09: 000000000000000c
> [  261.486970] R10: 000000000000000c R11: 0000000000000246 R12: 0000000000000001
> [  261.496599] R13: 000000000067b4e0 R14: 00007ffdc5c5248c R15: 00007ffdc5c52480
> [  261.506281] ==================================================================
> [  261.516076] Disabling lock debugging due to kernel taint
> [  261.523979] BUG: unable to handle kernel NULL pointer dereference at 00000000000000b0
> [  261.534413] #PF error: [normal kernel read fault]
> [  261.541730] PGD 8000000317400067 P4D 8000000317400067 PUD 316878067 PMD 0
> [  261.551294] Oops: 0000 [#1] SMP KASAN PTI
> [  261.557985] CPU: 14 PID: 2976 Comm: tc Tainted: G    B             5.0.0-rc7+ #157
> [  261.568306] Hardware name: Supermicro SYS-2028TP-DECR/X10DRT-P, BIOS 2.0b 03/30/2017
> [  261.578874] RIP: 0010:dst_cache_destroy+0x21/0xa0
> [  261.586413] Code: f4 ff ff ff eb f6 0f 1f 00 0f 1f 44 00 00 41 56 41 55 49 c7 c6 60 fe 35 af 41 54 55 49 89 fc 53 bd ff ff ff ff e8 ef 98 73 ff <49> 83 3c 24 00 75 35 eb 6c 4c 63 ed e8 de 98 73 ff 4a 8d 3c ed 40
> [  261.611247] RSP: 0018:ffff888316447160 EFLAGS: 00010282
> [  261.619564] RAX: 0000000000000000 RBX: ffff88835b3e2f00 RCX: ffffffffad1c5071
> [  261.629862] RDX: 0000000000000003 RSI: dffffc0000000000 RDI: 0000000000000297
> [  261.640149] RBP: 00000000ffffffff R08: fffffbfff5dd4e89 R09: fffffbfff5dd4e89
> [  261.650467] R10: 0000000000000001 R11: fffffbfff5dd4e88 R12: 00000000000000b0
> [  261.660785] R13: ffff8883267a10c0 R14: ffffffffaf35fe60 R15: 0000000000000001
> [  261.671110] FS:  00007f28ea3e6400(0000) GS:ffff888364200000(0000) knlGS:0000000000000000
> [  261.682447] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [  261.691491] CR2: 00000000000000b0 CR3: 00000003178ae004 CR4: 00000000001606e0
> [  261.701283] Call Trace:
> [  261.706374]  tunnel_key_release+0x3a/0x50 [act_tunnel_key]
> [  261.714522]  tcf_action_cleanup+0x2c/0xc0
> [  261.721208]  tcf_generic_walker+0x4c2/0x5c0
> [  261.728074]  ? tcf_action_dump_1+0x390/0x390
> [  261.734996]  ? tunnel_key_walker+0x5/0x1a0 [act_tunnel_key]
> [  261.743247]  ? tunnel_key_walker+0xe9/0x1a0 [act_tunnel_key]
> [  261.751557]  tca_action_gd+0x600/0xa40
> [  261.757991]  ? tca_get_fill.constprop.17+0x200/0x200
> [  261.765644]  ? __lock_acquire+0x588/0x1d20
> [  261.772461]  ? __lock_acquire+0x588/0x1d20
> [  261.779266]  ? mark_held_locks+0x90/0x90
> [  261.785880]  ? mark_held_locks+0x90/0x90
> [  261.792470]  ? __nla_parse+0xfe/0x190
> [  261.798738]  tc_ctl_action+0x218/0x230
> [  261.805145]  ? tcf_action_add+0x230/0x230
> [  261.811760]  rtnetlink_rcv_msg+0x3a5/0x600
> [  261.818564]  ? lock_downgrade+0x2d0/0x2d0
> [  261.825433]  ? validate_linkmsg+0x400/0x400
> [  261.832256]  ? find_held_lock+0x6d/0xd0
> [  261.838624]  ? match_held_lock+0x1b/0x210
> [  261.845142]  ? validate_linkmsg+0x400/0x400
> [  261.851729]  netlink_rcv_skb+0xc7/0x1f0
> [  261.857976]  ? netlink_ack+0x470/0x470
> [  261.864132]  ? netlink_deliver_tap+0x1f3/0x5a0
> [  261.870969]  netlink_unicast+0x2ae/0x350
> [  261.877294]  ? netlink_attachskb+0x340/0x340
> [  261.883962]  ? _copy_from_iter_full+0xdd/0x380
> [  261.890750]  ? __virt_addr_valid+0xb6/0xf0
> [  261.897188]  ? __check_object_size+0x159/0x240
> [  261.903928]  netlink_sendmsg+0x4d3/0x630
> [  261.910112]  ? netlink_unicast+0x350/0x350
> [  261.916410]  ? netlink_unicast+0x350/0x350
> [  261.922656]  sock_sendmsg+0x6d/0x80
> [  261.928257]  ___sys_sendmsg+0x48e/0x540
> [  261.934183]  ? copy_msghdr_from_user+0x210/0x210
> [  261.940865]  ? save_stack+0x89/0xb0
> [  261.946355]  ? __lock_acquire+0x588/0x1d20
> [  261.952358]  ? entry_SYSCALL_64_after_hwframe+0x49/0xbe
> [  261.959468]  ? mark_held_locks+0x90/0x90
> [  261.965248]  ? do_filp_open+0x138/0x1d0
> [  261.970910]  ? may_open_dev+0x50/0x50
> [  261.976386]  ? match_held_lock+0x1b/0x210
> [  261.982210]  ? __fget_light+0xa6/0xe0
> [  261.987648]  ? __sys_sendmsg+0xd2/0x150
> [  261.993263]  __sys_sendmsg+0xd2/0x150
> [  261.998613]  ? __ia32_sys_shutdown+0x30/0x30
> [  262.004555]  ? lock_downgrade+0x2d0/0x2d0
> [  262.010236]  ? mark_held_locks+0x1c/0x90
> [  262.015758]  ? do_syscall_64+0x1e/0x280
> [  262.021234]  do_syscall_64+0x78/0x280
> [  262.026500]  entry_SYSCALL_64_after_hwframe+0x49/0xbe
> [  262.033207] RIP: 0033:0x7f28e91a8b87
> [  262.038421] Code: 64 89 02 48 c7 c0 ff ff ff ff eb b9 0f 1f 80 00 00 00 00 8b 05 6a 2b 2c 00 48 63 d2 48 63 ff 85 c0 75 18 b8 2e 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 59 f3 c3 0f 1f 80 00 00 00 00 53 48 89 f3 48
> [  262.060708] RSP: 002b:00007ffdc5c4e2d8 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
> [  262.070112] RAX: ffffffffffffffda RBX: 000000005c73c202 RCX: 00007f28e91a8b87
> [  262.079087] RDX: 0000000000000000 RSI: 00007ffdc5c4e340 RDI: 0000000000000003
> [  262.088122] RBP: 0000000000000000 R08: 0000000000000001 R09: 000000000000000c
> [  262.097157] R10: 000000000000000c R11: 0000000000000246 R12: 0000000000000001
> [  262.106207] R13: 000000000067b4e0 R14: 00007ffdc5c5248c R15: 00007ffdc5c52480
> [  262.115271] Modules linked in: act_tunnel_key act_skbmod act_simple act_connmark nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 act_csum libcrc32c act_meta_skbtcindex act_meta_skbprio act_meta_mark act_ife ife act_police act_sample psample act_gact veth nfsv3 nfs_acl nfs lockd grace fscache bridge stp llc intel_rapl sb_edac mlx5_ib x86_pkg_temp_thermal sunrpc intel_powerclamp coretemp ib_uverbs kvm_intel ib_core kvm irqbypass mlx5_core crct10dif_pclmul crc32_pclmul crc32c_intel igb ghash_clmulni_intel intel_cstate mlxfw iTCO_wdt devlink intel_uncore iTCO_vendor_support ipmi_ssif ptp mei_me intel_rapl_perf ioatdma joydev pps_core ses mei i2c_i801 pcspkr enclosure lpc_ich dca wmi ipmi_si ipmi_devintf ipmi_msghandler acpi_pad acpi_power_meter pcc_cpufreq ast i2c_algo_bit drm_kms_helper ttm drm mpt3sas raid_class scsi_transport_sas
> [  262.204393] CR2: 00000000000000b0
> [  262.210390] ---[ end trace 2e41d786f2c7901a ]---
> [  262.226790] RIP: 0010:dst_cache_destroy+0x21/0xa0
> [  262.234083] Code: f4 ff ff ff eb f6 0f 1f 00 0f 1f 44 00 00 41 56 41 55 49 c7 c6 60 fe 35 af 41 54 55 49 89 fc 53 bd ff ff ff ff e8 ef 98 73 ff <49> 83 3c 24 00 75 35 eb 6c 4c 63 ed e8 de 98 73 ff 4a 8d 3c ed 40
> [  262.258311] RSP: 0018:ffff888316447160 EFLAGS: 00010282
> [  262.266304] RAX: 0000000000000000 RBX: ffff88835b3e2f00 RCX: ffffffffad1c5071
> [  262.276251] RDX: 0000000000000003 RSI: dffffc0000000000 RDI: 0000000000000297
> [  262.286208] RBP: 00000000ffffffff R08: fffffbfff5dd4e89 R09: fffffbfff5dd4e89
> [  262.296183] R10: 0000000000000001 R11: fffffbfff5dd4e88 R12: 00000000000000b0
> [  262.306157] R13: ffff8883267a10c0 R14: ffffffffaf35fe60 R15: 0000000000000001
> [  262.316139] FS:  00007f28ea3e6400(0000) GS:ffff888364200000(0000) knlGS:0000000000000000
> [  262.327146] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [  262.335815] CR2: 00000000000000b0 CR3: 00000003178ae004 CR4: 00000000001606e0
> 
> Fixes: 41411e2fd6b8 ("net/sched: act_tunnel_key: Add dst_cache support")
> Signed-off-by: Vlad Buslov <vladbu@...lanox.com>
> ---
> Changes from V1 to V2:
> - Extract metadata->dst error handler fix into standalone patch that targets
>   net.
> 
>  net/sched/act_tunnel_key.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/net/sched/act_tunnel_key.c b/net/sched/act_tunnel_key.c
> index 3404af72b4c1..fc2b884b170c 100644
> --- a/net/sched/act_tunnel_key.c
> +++ b/net/sched/act_tunnel_key.c
> @@ -201,8 +201,14 @@ static void tunnel_key_release_params(struct tcf_tunnel_key_params *p)
>  {
>  	if (!p)
>  		return;
> -	if (p->tcft_action == TCA_TUNNEL_KEY_ACT_SET)
> +	if (p->tcft_action == TCA_TUNNEL_KEY_ACT_SET) {
> +#ifdef CONFIG_DST_CACHE
> +		struct ip_tunnel_info *info = &p->tcft_enc_metadata->u.tun_info;
> +
> +		dst_cache_destroy(&info->dst_cache);
> +#endif
>  		dst_release(&p->tcft_enc_metadata->dst);
> +	}
>  	kfree_rcu(p, rcu);
>  }
>  
> @@ -384,7 +390,8 @@ static int tunnel_key_init(struct net *net, struct nlattr *nla,
>  
>  release_dst_cache:
>  #ifdef CONFIG_DST_CACHE
> -	dst_cache_destroy(&metadata->u.tun_info.dst_cache);
> +	if (metadata)
> +		dst_cache_destroy(&metadata->u.tun_info.dst_cache);
>  #endif
>  release_tun_meta:
>  	dst_release(&metadata->dst);
> @@ -401,15 +408,8 @@ static void tunnel_key_release(struct tc_action *a)
>  {
>  	struct tcf_tunnel_key *t = to_tunnel_key(a);
>  	struct tcf_tunnel_key_params *params;
> -#ifdef CONFIG_DST_CACHE
> -	struct ip_tunnel_info *info;
> -#endif
>  
>  	params = rcu_dereference_protected(t->params, 1);
> -#ifdef CONFIG_DST_CACHE
> -	info = &params->tcft_enc_metadata->u.tun_info;
> -	dst_cache_destroy(&info->dst_cache);
> -#endif
>  	tunnel_key_release_params(params);
>  }
>  
> 

Reviewed-by: Roi Dayan <roid@...lanox.com>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ