Re: [PATCH net] netfilter: nfnetlink_log: initialize nfgenmsg in NLMSG_DONE terminator

[Florian Westphal <fw@strlen.de>]

Xiang Mei <xmei5@asu.edu> wrote:
> When batching multiple NFLOG messages (inst->qlen > 1), __nfulnl_send()
> appends an NLMSG_DONE terminator with sizeof(struct nfgenmsg) payload via
> nlmsg_put(), but never initializes the nfgenmsg bytes. The nlmsg_put()
> helper only zeroes alignment padding after the payload, not the payload
> itself, so four bytes of stale kernel heap data are leaked to userspace
> in the NLMSG_DONE message body.
> 
> Initialize the nfgenmsg struct after nlmsg_put(), consistent with how
> __build_packet_message() populates nfgenmsg for regular NFULNL_MSG_PACKET
> messages, to prevent leaking kernel heap data to userspace.
> 
> Fixes: 29c5d4afba51 ("[NETFILTER]: nfnetlink_log: fix sending of multipart messages")
> Reported-by: Weiming Shi <bestswngs@gmail.com>
> Signed-off-by: Xiang Mei <xmei5@asu.edu>
> ---
>  net/netfilter/nfnetlink_log.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/net/netfilter/nfnetlink_log.c b/net/netfilter/nfnetlink_log.c
> index fcbe54940b2e..ad4eaf27590e 100644
> --- a/net/netfilter/nfnetlink_log.c
> +++ b/net/netfilter/nfnetlink_log.c
> @@ -361,6 +361,7 @@ static void
>  __nfulnl_send(struct nfulnl_instance *inst)
>  {
>  	if (inst->qlen > 1) {
> +		struct nfgenmsg *nfmsg;
>  		struct nlmsghdr *nlh = nlmsg_put(inst->skb, 0, 0,
>  						 NLMSG_DONE,
>  						 sizeof(struct nfgenmsg),

Would you mind sending a v2 that replaces nlmsg_put with nfnl_msg_put() ?

We already use this helper in __build_packet_message() and it takes
care of initialising the nfgenmsg.