Zero cmsg alignment bytes to avoid passing uninitialized bytes to kernel
CMSG_SPACE() might add alignment bytes either after the cmsg header or
after the cmsg data. In this case, on an x86_64 machine four bytes were
added to the cmsg data (CMSG_LEN(): 36, CMSG_SPACE(): 40), leading to
the following valgrind report:
$ valgrind --track-origins=yes ./src/pim6sd -f ./pim6sd.conf
[...]
==25485== Syscall param sendmsg(msg.msg_control) points to uninitialised byte(s)
==25485== at 0x4964914: sendmsg (sendmsg.c:28)
==25485== by 0x11C1DF: send_pim6 (pim6.c:449)
==25485== by 0x11CE01: send_pim6_hello (pim6_proto.c:824)
==25485== by 0x129A27: start_all_vifs (vif.c:298)
==25485== by 0x10A73D: main (main.c:327)
==25485== Address 0x4a75474 is 36 bytes inside a block of size 40 alloc'd
==25485== at 0x483577F: malloc (vg_replace_malloc.c:299)
==25485== by 0x11BF99: init_pim6 (pim6.c:218)
==25485== by 0x10A729: main (main.c:316)
==25485== Uninitialised value was created by a heap allocation
==25485== at 0x483577F: malloc (vg_replace_malloc.c:299)
==25485== by 0x11BF99: init_pim6 (pim6.c:218)
==25485== by 0x10A729: main (main.c:316)
==25485==
08:12:27.578 Local BSR address: fd5c:725:2841:4203::2
08:12:27.579 Local BSR priority : 0
08:12:27.579 Local BSR period is : 0 sec.
08:12:27.580 Local BSR hash mask length: 0
08:12:27.593 skip the existing address
==25485== Syscall param sendmsg(msg.msg_control) points to uninitialised byte(s)
==25485== at 0x4964914: sendmsg (sendmsg.c:28)
==25485== by 0x11C1DF: send_pim6 (pim6.c:449)
==25485== by 0x11CE01: send_pim6_hello (pim6_proto.c:824)
==25485== by 0x120C4A: receive_pim6_hello (pim6_proto.c:351)
==25485== by 0x11BAE4: accept_pim6 (pim6.c:347)
==25485== by 0x11BAE4: pim6_read (pim6.c:268)
==25485== by 0x10AB0F: main (main.c:535)
==25485== Address 0x4a75474 is 36 bytes inside a block of size 40 alloc'd
==25485== at 0x483577F: malloc (vg_replace_malloc.c:299)
==25485== by 0x11BF99: init_pim6 (pim6.c:218)
==25485== by 0x10A729: main (main.c:316)
==25485== Uninitialised value was created by a heap allocation
==25485== at 0x483577F: malloc (vg_replace_malloc.c:299)
==25485== by 0x11BF99: init_pim6 (pim6.c:218)
==25485== by 0x10A729: main (main.c:316)
==25485==
[...]
Fixing this by always zeroing the sndcmsgbufpim buffer.
Which then results in the following complaint by valgrind even though
all members of the original struct were explictly set:
$ valgrind --track-origins=yes ./src/pim6sd -f ./pim6sd.conf
[...]
==10491== Syscall param socketcall.setsockopt(optval) points to uninitialised byte(s)
==10491== at 0x4964A6A: setsockopt (syscall-template.S:78)
==10491== by 0x11474D: k_chg_mfc (kern.c:360)
==10491== by 0x123F46: change_interfaces.part.0 (route.c:849)
==10491== by 0x123DF5: change_interfaces.part.0 (route.c:760)
==10491== by 0x11ED6D: receive_pim6_join_prune (pim6_proto.c:2371)
==10491== by 0x11BB5F: accept_pim6 (pim6.c:357)
==10491== by 0x11BB5F: pim6_read (pim6.c:269)
==10491== by 0x10AB0F: main (main.c:535)
==10491== Address 0x1ffefff8fa is on thread 1's stack
==10491== in frame #1, created by k_chg_mfc (kern.c:330)
==10491== Uninitialised value was created by a stack allocation
==10491== at 0x114660: k_chg_mfc (kern.c:330)
==10491==
11:25:38.118 Join/Prune timer expired
11:25:38.119 Join_or_prune : upstream_router is null
11:25:38.120 grp_action = 0
11:25:38.121 Join_or_prune : upstream_router is null
11:25:38.121 src_action = 0
---------------------------RP-Set----------------------------
Current BSR address: :: Prio: 0 Timeout: 145
RP-address(Upstream)/Group prefix Prio Hold Age
fd5c:725:2841:4203::2(myself)
ff7e:240:fd5c:725:2841:4203::/96 1 65535 65535
==10491== Syscall param socketcall.setsockopt(optval) points to uninitialised byte(s)
==10491== at 0x4964A6A: setsockopt (syscall-template.S:78)
==10491== by 0x11474D: k_chg_mfc (kern.c:360)
==10491== by 0x10AB0F: main (main.c:535)
==10491== Address 0x1ffeffff0a is on thread 1's stack
==10491== in frame #1, created by k_chg_mfc (kern.c:330)
==10491== Uninitialised value was created by a stack allocation
==10491== at 0x124B95: process_cache_miss (route.c:1084)
==10491== by 0x124B95: process_kernel_call (route.c:924)
==10491==
[...]
Silencing valgrind by zeroing the full struct via memset before using
it.
"struct mf6cctl" not only contains two but four variables. k_del_mfc()
misses to initialize "mifi_t mf6cc_parent" and
"struct if_set mf6cc_ifset", resulting in the following valgrind report
on startup:
$ valgrind --track-origins=yes ./src/pim6sd -f ./pim6sd.conf
[...]
==5847== Syscall param socketcall.setsockopt(optval) points to uninitialised byte(s)
==5847== at 0x4964A6A: setsockopt (syscall-template.S:78)
==5847== by 0x1145DC: k_del_mfc (kern.c:304)
==5847== by 0x119C5D: delete_mrtentry_all_kernel_cache.part.4 (mrt.c:1218)
==5847== by 0x11A1CC: delete_mrtentry_all_kernel_cache (mrt.c:1207)
==5847== by 0x11A1CC: delete_mrtentry (mrt.c:656)
==5847== by 0x1271C4: age_routes (timer.c:1218)
==5847== by 0x11494D: timer (main.c:577)
==5847== by 0x10AF09: age_callout_queue (callout.c:130)
==5847== by 0x10AA55: main (main.c:523)
==5847== Address 0x1ffefffd98 is on thread 1's stack
==5847== in frame #1, created by k_del_mfc (kern.c:297)
==5847== Uninitialised value was created by a stack allocation
==5847== at 0x114580: k_del_mfc (kern.c:297)
[...]
Fixing this zeroing the full struct mf6cctl. This should also make it
future-proof to any new variables added in the future.
It seems that at the moment these variables are unused for a
MRT6_DEL_MFC in the Linux kernel. So in practice no harm should have
been caused previously.
"struct mif6ctl" not only contains three but five variables. k_add_if()
misses to initialize "unsigned char vifc_threshold" and
"unsigned int vifc_rate_limit", resulting in the following valgrind
report on startup:
$ valgrind --track-origins=yes ./src/pim6sd -f ./pim6sd.conf
[...]
==25485== Syscall param socketcall.setsockopt(optval) points to uninitialised byte(s)
==25485== at 0x4964A6A: setsockopt (syscall-template.S:78)
==25485== by 0x11449D: k_add_vif (kern.c:272)
==25485== by 0x12976F: start_vif (vif.c:329)
==25485== by 0x129A27: start_all_vifs (vif.c:298)
==25485== by 0x10A73D: main (main.c:327)
==25485== Address 0x1ffefffeb7 is on thread 1's stack
==25485== in frame #1, created by k_add_vif (kern.c:260)
==25485== Uninitialised value was created by a stack allocation
==25485== at 0x114450: k_add_vif (kern.c:260)
[...]
Fixing this zeroing the full struct mif6ctl. This should also make it
future-proof to any new variables added in the future.
It seems that at the moment these variables are unused for a
MRT6_ADD_MIF in the Linux kernel. So in practice no harm should have
been caused so far.
When using embedded-RP addresses pim6sd would sometimes crash on the
first call to age_routes(). valgrind was able to track this down to the
following use-after-free issue:
$ valgrind --track-origins=yes ./src/pim6sd -f ./pim6sd.conf
[...]
04:20:42.949 warning - setsockopt MRT6_DEL_MFC: No such file or directory
==2722== Invalid read of size 8
==2722== at 0x1271A9: age_routes (timer.c:673)
==2722== by 0x11492D: timer (main.c:577)
==2722== by 0x10AF09: age_callout_queue (callout.c:130)
==2722== by 0x10AA55: main (main.c:523)
==2722== Address 0x4a95240 is 0 bytes inside a block of size 64 free'd
==2722== at 0x48369AB: free (vg_replace_malloc.c:530)
==2722== by 0x119F9B: delete_grpentry (mrt.c:611)
==2722== by 0x11A26C: delete_mrtentry (mrt.c:701)
==2722== by 0x127184: age_routes (timer.c:1215)
==2722== by 0x11492D: timer (main.c:577)
==2722== by 0x10AF09: age_callout_queue (callout.c:130)
==2722== by 0x10AA55: main (main.c:523)
==2722== Block was alloc'd at
==2722== at 0x483577F: malloc (vg_replace_malloc.c:299)
==2722== by 0x12649A: add_rp_grp_entry (rp.c:658)
==2722== by 0x1258A5: rp_grp_match (rp.c:1274)
==2722== by 0x11AE27: find_route (mrt.c:294)
==2722== by 0x124981: process_cache_miss (route.c:1007)
==2722== by 0x124981: process_kernel_call (route.c:924)
==2722== by 0x10AB0F: main (main.c:535)
==2722==
==2722== Invalid read of size 8
==2722== at 0x1271BF: age_routes (timer.c:424)
==2722== by 0x11492D: timer (main.c:577)
==2722== by 0x10AF09: age_callout_queue (callout.c:130)
==2722== by 0x10AA55: main (main.c:523)
==2722== Address 0x4a95140 is 0 bytes inside a block of size 32 free'd
==2722== at 0x48369AB: free (vg_replace_malloc.c:530)
==2722== by 0x125F6A: delete_rp_grp_entry (rp.c:756)
==2722== by 0x119F9B: delete_grpentry (mrt.c:611)
==2722== by 0x11A26C: delete_mrtentry (mrt.c:701)
==2722== by 0x127184: age_routes (timer.c:1215)
==2722== by 0x11492D: timer (main.c:577)
==2722== by 0x10AF09: age_callout_queue (callout.c:130)
==2722== by 0x10AA55: main (main.c:523)
==2722== Block was alloc'd at
==2722== at 0x483577F: malloc (vg_replace_malloc.c:299)
==2722== by 0x12645A: add_cand_rp (rp.c:409)
==2722== by 0x12645A: add_rp_grp_entry (rp.c:586)
==2722== by 0x1258A5: rp_grp_match (rp.c:1274)
==2722== by 0x11AE27: find_route (mrt.c:294)
==2722== by 0x124981: process_cache_miss (route.c:1007)
==2722== by 0x124981: process_kernel_call (route.c:924)
==2722== by 0x10AB0F: main (main.c:535)
[...]
Storing the affected two next pointers before the cand_rp_t and
rp_grp_entry_t pointers are free()'d fixes the issue.
