netdev - Re: [PATCH net-next, v5] netlink: add IGMP/MLD join/leave notifications

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: <20241209182549.271ede3a@kernel.org>
Date: Mon, 9 Dec 2024 18:25:49 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: Yuyang Huang <yuyanghuang@...gle.com>
Cc: "David S. Miller" <davem@...emloft.net>, Eric Dumazet
 <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>, Simon Horman
 <horms@...nel.org>, David Ahern <dsahern@...nel.org>,
 roopa@...ulusnetworks.com, jiri@...nulli.us, stephen@...workplumber.org,
 jimictw@...gle.com, prohr@...gle.com, liuhangbin@...il.com,
 nicolas.dichtel@...nd.com, andrew@...n.ch, netdev@...r.kernel.org, "Maciej
 Żenczykowski" <maze@...gle.com>, Lorenzo Colitti
 <lorenzo@...gle.com>, Patrick Ruddy <pruddy@...tta.att-mail.com>
Subject: Re: [PATCH net-next, v5] netlink: add IGMP/MLD join/leave
 notifications

On Fri,  6 Dec 2024 13:10:25 +0900 Yuyang Huang wrote:
> +static int inet6_fill_ifmcaddr(struct sk_buff *skb, struct net_device *dev,
> +			       const struct in6_addr *addr, int event)
> +{
> +	struct ifaddrmsg *ifm;
> +	struct nlmsghdr *nlh;
> +
> +	nlh = nlmsg_put(skb, 0, 0, event, sizeof(struct ifaddrmsg), 0);
> +	if (!nlh)
> +		return -EMSGSIZE;
> +
> +	ifm = nlmsg_data(nlh);
> +	ifm->ifa_family = AF_INET6;
> +	ifm->ifa_prefixlen = 128;
> +	ifm->ifa_flags = IFA_F_PERMANENT;
> +	ifm->ifa_scope = RT_SCOPE_UNIVERSE;
> +	ifm->ifa_index = dev->ifindex;
> +
> +	if (nla_put_in6_addr(skb, IFA_MULTICAST, addr) < 0) {
> +		nlmsg_cancel(skb, nlh);
> +		return -EMSGSIZE;
> +	}
> +
> +	nlmsg_end(skb, nlh);
> +	return 0;
> +}

Is there a strong reason you reimplement this instead of trying to reuse
inet6_fill_ifmcaddr() ? Keeping notifications and get responses in sync
used to be a thing in rtnetlink, this code already diverged but maybe
we can bring it back.

> +static void inet6_ifmcaddr_notify(struct net_device *dev,
> +				  const struct in6_addr *addr, int event)
> +{
> +	struct net *net = dev_net(dev);
> +	struct sk_buff *skb;
> +	int err = -ENOBUFS;

ENOMEM ? I could be wrong but in atomic context the memory pressure 
can well be transient, it's not like the socket queue filled up.

> +	skb = nlmsg_new(NLMSG_ALIGN(sizeof(struct ifaddrmsg))
> +			+ nla_total_size(16), GFP_ATOMIC);

nit: + goes to the end of previous line

> +	if (!skb)
> +		goto error;
> +
> +	err = inet6_fill_ifmcaddr(skb, dev, addr, event);
> +	if (err < 0) {
> +		WARN_ON_ONCE(err == -EMSGSIZE);
> +		kfree_skb(skb);

nit: nlmsg_free(), since it exists

> +		goto error;
> +	}
> +
> +	rtnl_notify(skb, net, 0, RTNLGRP_IPV6_MCADDR, NULL, GFP_ATOMIC);
> +	return;
> +error:
> +	rtnl_set_sk_err(net, RTNLGRP_IPV6_MCADDR, err);
> +}
> +
-- 
pw-bot: cr