netdev - Re: [PATCH net-next v2] mctp: emit RTM_NEWADDR and RTM

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20211217114247.39b632c8@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Fri, 17 Dec 2021 11:42:54 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Matt Johnston <matt@...econstruct.com.au>
Cc:     Jeremy Kerr <jk@...econstruct.com.au>, netdev@...r.kernel.org,
        "David S. Miller" <davem@...emloft.net>
Subject: Re: [PATCH net-next v2] mctp: emit RTM_NEWADDR and RTM_DELADDR

On Wed, 15 Dec 2021 13:32:50 +0800 Matt Johnston wrote:
> Userspace can receive notification of MCTP address changes via
> RTNLGRP_MCTP_IFADDR rtnetlink multicast group.
> 
> Signed-off-by: Matt Johnston <matt@...econstruct.com.au>
> ---
> 
> v2: Simplify error return path, fix local variable ordering
> 
> I've left req_skb as-is rather than passing portid, as noted 
> it keeps callers tidier.

Sorry for late review.

> -static int mctp_fill_addrinfo(struct sk_buff *skb, struct netlink_callback *cb,
> -			      struct mctp_dev *mdev, mctp_eid_t eid)
> +static int mctp_addrinfo_size(void)
> +{
> +	return NLMSG_ALIGN(sizeof(struct ifaddrmsg))
> +		+ nla_total_size(4) // IFA_LOCAL
> +		+ nla_total_size(4) // IFA_ADDRESS

why 4? The attributes are u8, no?

> +		;
> +}

> @@ -127,6 +141,30 @@ static int mctp_dump_addrinfo(struct sk_buff *skb, struct netlink_callback *cb)
>  	return skb->len;
>  }
>  
> +static void mctp_addr_notify(struct mctp_dev *mdev, mctp_eid_t eid, int msg_type,
> +			     struct sk_buff *req_skb, struct nlmsghdr *req_nlh)
> +{
> +	u32 portid = NETLINK_CB(req_skb).portid;
> +	struct net *net = dev_net(mdev->dev);
> +	struct sk_buff *skb;
> +	int rc = -ENOBUFS;
> +
> +	skb = nlmsg_new(mctp_addrinfo_size(), GFP_KERNEL);
> +	if (!skb)
> +		goto out;
> +
> +	rc = mctp_fill_addrinfo(skb, mdev, eid, msg_type,
> +				portid, req_nlh->nlmsg_seq, 0);
> +	if (rc < 0)

How about a customary WARN_ON_ONCE(rc == -EMSGSIZE) here?

> +		goto out;
> +
> +	rtnl_notify(skb, net, portid, RTNLGRP_MCTP_IFADDR, req_nlh, GFP_KERNEL);
> +	return;
> +out:
> +	kfree_skb(skb);
> +	rtnl_set_sk_err(net, RTNLGRP_MCTP_IFADDR, rc);
> +}