ok mvs@
> On 14 Dec 2021, at 16:49, Alexander Bluhm <[email protected]> wrote:
>
> Hi,
>
> syzkaller found a NULL dereference:
>
> https://syzkaller.appspot.com/bug?id=a7f159c677ec125fe9edef2265e2749f13e24243
>
> It looks like inm->inm_rti is NULL. It is set in rti_fill() or not
> set if malloc(9) fails. There is no rollback if malloc fails so
> the field stays uninitialized.
>
> The code is called from ioctl, setsockopt or a task. Malloc should
> wait instead of failing, otherwise syscalls would be unreliable.
> While there also put an M_WAIT in the init code. During init malloc
> must not fail.
>
> ok?
>
> bluhm
>
> Index: netinet/igmp.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/igmp.c,v
> retrieving revision 1.76
> diff -u -p -r1.76 igmp.c
> --- netinet/igmp.c 17 Aug 2020 16:25:34 -0000 1.76
> +++ netinet/igmp.c 14 Dec 2021 13:07:22 -0000
> @@ -117,11 +117,7 @@ igmp_init(void)
> LIST_INIT(&rti_head);
>
> igmpcounters = counters_alloc(igps_ncounters);
> - router_alert = m_get(M_DONTWAIT, MT_DATA);
> - if (router_alert == NULL) {
> - printf("%s: no mbuf\n", __func__);
> - return;
> - }
> + router_alert = m_get(M_WAIT, MT_DATA);
>
> /*
> * Construct a Router Alert option (RAO) to use in report
> @@ -142,7 +138,6 @@ igmp_init(void)
> router_alert->m_len = sizeof(ra->ipopt_dst) + ra->ipopt_list[1];
> }
>
> -/* Return -1 for error. */
> int
> rti_fill(struct in_multi *inm)
> {
> @@ -158,9 +153,7 @@ rti_fill(struct in_multi *inm)
> }
> }
>
> - rti = malloc(sizeof(*rti), M_MRTABLE, M_NOWAIT);
> - if (rti == NULL)
> - return (-1);
> + rti = malloc(sizeof(*rti), M_MRTABLE, M_WAITOK);
> rti->rti_ifidx = inm->inm_ifidx;
> rti->rti_type = IGMP_v2_ROUTER;
> LIST_INSERT_HEAD(&rti_head, rti, rti_list);
> @@ -501,9 +494,7 @@ igmp_joingroup(struct in_multi *inm)
>
> if (!IN_LOCAL_GROUP(inm->inm_addr.s_addr) &&
> ifp && (ifp->if_flags & IFF_LOOPBACK) == 0) {
> - if ((i = rti_fill(inm)) == -1)
> - goto out;
> -
> + i = rti_fill(inm);
> igmp_sendpkt(ifp, inm, i, 0);
> inm->inm_state = IGMP_DELAYING_MEMBER;
> inm->inm_timer = IGMP_RANDOM_DELAY(
> @@ -512,7 +503,6 @@ igmp_joingroup(struct in_multi *inm)
> } else
> inm->inm_timer = 0;
>
> -out:
> if_put(ifp);
> }
>
>
