ipmi_enum_sp can lead to heap corruption

[rmustacc]

A user was booting on an HP DL360 Gen 9 and saw that syseventd was crashing on startup. Here was the stack trace:

> $C
fd57f4e8 libc.so.1`memset+0x10c(fc7142ee, 2c, fd57f538, feee0981)
fd57f518 libtopo.so.1`topo_zalloc+0x33(2c, 0)
fd57f538 libtopo.so.1`topo_hdl_zalloc+0x14(8119f40, 2c)
fd57f558 libtopo.so.1`topo_mod_zalloc+0x20(8120af8, 2c)
fd57f5a8 libtopo.so.1`tf_rdata_new+0x52(8120af8, 8224bf0, 8139d40, 821d8a0)
fd57f5f8 libtopo.so.1`topo_xml_walk+0x24a(8120af8, 8224bf0, 8138b80, 821d8a0)
fd57f638 libtopo.so.1`dependent_create+0x93(8120af8, 8224bf0, 821f500, 8138b80, 821d8a0)
fd57f678 libtopo.so.1`dependents_create+0x6c(8120af8, 8224bf0, 821f500, 81360a0, 821d8a0)
fd57f728 libtopo.so.1`pad_process+0xb0(8120af8, 811af68, 81360a0, 821d8a0, 811af90)
fd57f778 libtopo.so.1`topo_xml_range_process+0x353(8120af8, 81360a0, 811af68)
fd57f7c8 libtopo.so.1`tf_rdata_new+0x118(8120af8, 8224bf0, 81360a0, 821d900)
fd57f818 libtopo.so.1`topo_xml_walk+0x24a(8120af8, 8224bf0, 8135f10, 821d900)
fd57f848 libtopo.so.1`topo_xml_enum+0x48(8120af8, 8224bf0, 821d900)
fd57f998 libtopo.so.1`topo_file_load+0xe6(8120af8, 821d900, fcc0dfe8, fcc0dc4d, 0)
fd57f9c8 libtopo.so.1`topo_mod_enummap+0x25(8120af8, 821d900, fcc0dfe8, fcc0dc4d)
fd57fa18 x86pi.so`x86pi_enum_start+0x157(8120af8, fd57fa4c)
fd57fa78 x86pi.so`x86pi_enum+0x63(8120af8, 821d900, 81feaf8, 0, 0, 0)
fd57fad8 libtopo.so.1`topo_mod_enumerate+0xb1(8120af8, 821d900, 808ef88, 81feaf8, 0, 0)
fd57fb38 libtopo.so.1`enum_run+0x89(8120b68, 81df728)
fd57fb78 libtopo.so.1`topo_xml_range_process+0x144(8120b68, 816e1c0, 81df728)
fd57fbc8 libtopo.so.1`tf_rdata_new+0x118(8120b68, 8224c40, 816e1c0, 821d900)
fd57fc18 libtopo.so.1`topo_xml_walk+0x24a(8120b68, 8224c40, 816e030, 821d900)
fd57fc48 libtopo.so.1`topo_xml_enum+0x48(8120b68, 8224c40, 821d900)
fd57fd98 libtopo.so.1`topo_file_load+0xe6(8120b68, 821d900, 811efd0, 81feb08, 0)
fd57fdd8 libtopo.so.1`topo_tree_enum+0x90(8119f40, 821f740)
fd57fe08 libtopo.so.1`topo_tree_enum_all+0x22(8119f40)
fd57fe68 libtopo.so.1`topo_snap_create+0xd4(8119f40, fd57feb8, 0)
fd57fe98 libtopo.so.1`topo_snap_hold+0xb3(8119f40, 0, fd57feb8)
fd57fed8 libzfs.so.1`libzfs_fru_refresh+0x6c(8085498)
fd57ff38 zfs_mod.so`zfs_deliver_update+0xb3(8068f00, fd57ff68, fd57ff88, feee0981, 8083100, 0)
fd57ff88 zfs_mod.so`zfs_deliver_event+0x16c(809ce20)
fd57ffc8 client_deliver_event_thr+0xee(8083100, 0, 0, 0)
fd57ffe8 libc.so.1`_thrp_setup+0x81(fec68a40)
fd57fff8 libc.so.1`_lwp_start(fec68a40, 0, 0, 0, 0, 0)
> fc7142ee::whatis
fc7142ee is ipmi.so`_topo_init, in /usr/lib/fm/topo/plugins/ipmi.so [fc710000,fc716000)

If we look at what are trying to memset, we'll see that it is program text! Further, if we go stack diving, we'll see that this was actually what was returned from umem_alloc by topo_zalloc. So the question is how did we get this heap corruption. Thankfully, this problem was dead reproducible, so we were able to set UMEM_DEBUG=default and run it. The good news, is that this provides the smoking gun:

> $C
fd56f0a8 libc.so.1`syscall+0x13(3, fec58bcc, 0, fee7d47a, 0, 0)
fd56f0f8 libc.so.1`thr_sigsetmask+0x1f2(2, fd56f130, 0)
fd56f118 libc.so.1`sigprocmask+0x40(2, fd56f130, 0)
fd56f158 libc.so.1`sigrelse+0x68(6)
fd56f178 libumem.so.1`umem_do_abort+0x38()
fd56f188 libumem.so.1`__umem_assert_failed(fda50818, fc7d0ef8, 0)
fd56f1d8 libumem.so.1`umem_error+0x343(3, 80ee010, fc7d0ef8)
fd56f218 libumem.so.1`umem_free+0x97(fc7d0ef8, 2e)
fd56f238 libtopo.so.1`topo_free+0x1d(fc7d0ef8, 2e)
fd56f258 libtopo.so.1`topo_hdl_free+0x15(812df40, fc7d0ef8, 2e)
fd56f278 libtopo.so.1`topo_mod_free+0x23(92cde80, fc7d0ef8, 2e)
fd56f3e8 ipmi.so`ipmi_enum_sp+0x254(92cde80, 8530f68)
fd56f458 ipmi.so`ipmi_enum+0x91(92cde80, 8530f68, 917a148, 0, 0, 0)
fd56f4b8 libtopo.so.1`topo_mod_enumerate+0xb1(92cde80, 8530f68, 85cbae8, 917a148, 0, 0)
fd56f518 libtopo.so.1`enum_run+0x89(814aa80, 93ce940)
fd56f558 libtopo.so.1`topo_xml_range_process+0x144(814aa80, 8169130, 93ce940)
fd56f5a8 libtopo.so.1`tf_rdata_new+0x118(814aa80, 8527ca0, 8169130, 8530f68)
fd56f5f8 libtopo.so.1`topo_xml_walk+0x24a(814aa80, 8527ca0, 8168358, 8530f68)
fd56f638 libtopo.so.1`dependent_create+0x93(814aa80, 8527ca0, 85180a8, 8168358, 8530f68)
fd56f678 libtopo.so.1`dependents_create+0x6c(814aa80, 8527ca0, 85180a8, 8165858, 8530f68)
fd56f728 libtopo.so.1`pad_process+0xb0(814aa80, 8131f80, 8165858, 8530f68, 8131fa8)
fd56f778 libtopo.so.1`topo_xml_range_process+0x353(814aa80, 8165858, 8131f80)
fd56f7c8 libtopo.so.1`tf_rdata_new+0x118(814aa80, 8527ca0, 8165858, 84e0010)
fd56f818 libtopo.so.1`topo_xml_walk+0x24a(814aa80, 8527ca0, 81a0f28, 84e0010)
fd56f848 libtopo.so.1`topo_xml_enum+0x48(814aa80, 8527ca0, 84e0010)
fd56f998 libtopo.so.1`topo_file_load+0xe6(814aa80, 84e0010, fcc0dfe8, fcc0dc4d, 0)
fd56f9c8 libtopo.so.1`topo_mod_enummap+0x25(814aa80, 84e0010, fcc0dfe8, fcc0dc4d)
fd56fa18 x86pi.so`x86pi_enum_start+0x157(814aa80, fd56fa4c)
fd56fa78 x86pi.so`x86pi_enum+0x63(814aa80, 84e0010, 8470170, 0, 0, 0)
fd56fad8 libtopo.so.1`topo_mod_enumerate+0xb1(814aa80, 84e0010, 80a2d08, 8470170, 0, 0)
fd56fb38 libtopo.so.1`enum_run+0x89(814ab00, 83f6b80)
fd56fb78 libtopo.so.1`topo_xml_range_process+0x144(814ab00, 819f620, 83f6b80)
fd56fbc8 libtopo.so.1`tf_rdata_new+0x118(814ab00, 8527d40, 819f620, 84e0010)
fd56fc18 libtopo.so.1`topo_xml_walk+0x24a(814ab00, 8527d40, 8531ed8, 84e0010)
fd56fc48 libtopo.so.1`topo_xml_enum+0x48(814ab00, 8527d40, 84e0010)
fd56fd98 libtopo.so.1`topo_file_load+0xe6(814ab00, 84e0010, 813efc8, 84701a0, 0)
fd56fdd8 libtopo.so.1`topo_tree_enum+0x90(812df40, 8518468)
fd56fe08 libtopo.so.1`topo_tree_enum_all+0x22(812df40)
fd56fe68 libtopo.so.1`topo_snap_create+0xd4(812df40, fd56feb8, 0)
fd56fe98 libtopo.so.1`topo_snap_hold+0xb3(812df40, 0, fd56feb8)
fd56fed8 libzfs.so.1`libzfs_fru_refresh+0x6c(80854d8)
fd56ff38 zfs_mod.so`zfs_deliver_update+0xb3(8068f00, fd56ff68, fd56ff88, feee0981, 8083100, 0)
fd56ff88 zfs_mod.so`zfs_deliver_event+0x16c(809d1c0)
fd56ffc8 client_deliver_event_thr+0xee(8083100, 0, 0, 0)
fd56ffe8 libc.so.1`_thrp_setup+0x81(fec58a40)
fd56fff8 libc.so.1`_lwp_start(fec58a40, 0, 0, 0, 0, 0)
> ::umem_status
Status:         ready and active
Concurrency:    32
Logs:           (inactive)
Message buffer:
umem allocator: invalid free: buffer not in cache
buffer=fc7e0ef8  bufctl=0  cache: umem_alloc_48
umem: heap corruption detected
> fc7e0ef8::whatis
fc7e0ef8 is in [ unknown ] [fc7e0000,fc7e1000)

So we were trying to free something that is garbage. If we look at the code here, there's a single set of freeing paths that always iterates over the lancfg.ilc_ipv6_nroutes even though not all of them have been initialized. That is problematic, but would have probably led to us trying to free a NULL pointer. That doesn't quite get us here. However, if any other goto out is taken before we initialize ipv6_routes, we'll try to free it and it's an uninitialized pointer on the stack. Even worse, because we assume it's a valid array, that'll wreak havoc. The solution here is to fix the free path to account for what's actually been allocated and what hasn't been. Simply relying on the lancfg isn't sufficient. While it is zeroed initially, there is a bunch that can fail between it being initialized by calling ipmi_lan_get_config() and the actual allocation of the routes.

[danmcd]

Tested by adding IPv6 routes to an IPMI I have control over. It makes fmtopo dump core without the fix, but with the fix it no longer dumps core and behaves like an IPMI with no IPv6 routes.