From: Cong Wang <[email protected]>
Date: Sat, 27 Jun 2020 00:12:24 -0700
> genl_family_rcv_msg_attrs_parse() reuses the global family->attrbuf
> when family->parallel_ops is false. However, family->attrbuf is not
> protected by any lock on the genl_family_rcv_msg_doit() code path.
>
> This leads to several different consequences, one of them is UAF,
> like the following:
>
> genl_family_rcv_msg_doit(): genl_start():
> genl_family_rcv_msg_attrs_parse()
> attrbuf = family->attrbuf
> __nlmsg_parse(attrbuf);
> genl_family_rcv_msg_attrs_parse()
> attrbuf = family->attrbuf
> __nlmsg_parse(attrbuf);
> info->attrs = attrs;
> cb->data = info;
>
> netlink_unicast_kernel():
> consume_skb()
> genl_lock_dumpit():
> genl_dumpit_info(cb)->attrs
>
> Note family->attrbuf is an array of pointers to the skb data, once
> the skb is freed, any dereference of family->attrbuf will be a UAF.
>
> Maybe we could serialize the family->attrbuf with genl_mutex too, but
> that would make the locking more complicated. Instead, we can just get
> rid of family->attrbuf and always allocate attrbuf from heap like the
> family->parallel_ops==true code path. This may add some performance
> overhead but comparing with taking the global genl_mutex, it still
> looks better.
>
> Fixes: 75cdbdd08900 ("net: ieee802154: have genetlink code to parse the attrs
> during dumpit")
> Fixes: 057af7071344 ("net: tipc: have genetlink code to parse the attrs
> during dumpit")
> Reported-and-tested-by: [email protected]
> Reported-and-tested-by: [email protected]
> Reported-and-tested-by: [email protected]
> Reported-and-tested-by: [email protected]
> Reported-and-tested-by: [email protected]
> Cc: Jiri Pirko <[email protected]>
> Signed-off-by: Cong Wang <[email protected]>
Applied and queued up for -stable, thank you.
