From af009c87b41ea1bb594d68e53d46ca050057e2db Mon Sep 17 00:00:00 2001 From: Denys Fedoryshchenko Date: Sat, 8 Aug 2026 17:45:35 +0300 Subject: ipoe: zero generic netlink requests before filling them in The request buffers are plain stack variables and only the nlmsghdr fields and genlmsghdr.cmd were ever assigned, so genlmsghdr.version and genlmsghdr.reserved reached the kernel holding whatever happened to be on the stack. Since 6.1 genetlink validates the reserved header fields of every command whose id is >= genl_family.resv_start_op, and ipoe sets that field to CTRL_CMD_GETPOLICY + 1, i.e. 11. IPOE_CMD_DEL_NET is 11, so ipoe_nl_del_net(), which runs on startup and on every config reload, is already rejected with EINVAL whenever that garbage is nonzero, and any command added after it is affected as well. --- accel-pppd/ctrl/ipoe/ipoe_netlink.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) (limited to 'accel-pppd') diff --git a/accel-pppd/ctrl/ipoe/ipoe_netlink.c b/accel-pppd/ctrl/ipoe/ipoe_netlink.c index 7057c91e..94b7f42a 100644 --- a/accel-pppd/ctrl/ipoe/ipoe_netlink.c +++ b/accel-pppd/ctrl/ipoe/ipoe_netlink.c @@ -46,6 +46,8 @@ int ipoe_nl_add_exclude(uint32_t addr, int mask) return -1; } + memset(&req, 0, sizeof(req)); + nlh = &req.n; nlh->nlmsg_len = NLMSG_LENGTH(GENL_HDRLEN); nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK; @@ -82,6 +84,8 @@ void ipoe_nl_del_exclude(uint32_t addr) return; } + memset(&req, 0, sizeof(req)); + nlh = &req.n; nlh->nlmsg_len = NLMSG_LENGTH(GENL_HDRLEN); nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK; @@ -114,6 +118,8 @@ int ipoe_nl_add_net(uint32_t addr, int mask) return -1; } + memset(&req, 0, sizeof(req)); + nlh = &req.n; nlh->nlmsg_len = NLMSG_LENGTH(GENL_HDRLEN); nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK; @@ -150,6 +156,8 @@ void ipoe_nl_del_net(uint32_t addr) return; } + memset(&req, 0, sizeof(req)); + nlh = &req.n; nlh->nlmsg_len = NLMSG_LENGTH(GENL_HDRLEN); nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK; @@ -181,6 +189,8 @@ void ipoe_nl_add_interface(int ifindex, uint8_t mode) return; } + memset(&req, 0, sizeof(req)); + nlh = &req.n; nlh->nlmsg_len = NLMSG_LENGTH(GENL_HDRLEN); nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK; @@ -213,6 +223,8 @@ void ipoe_nl_del_interface(int ifindex) return; } + memset(&req, 0, sizeof(req)); + nlh = &req.n; nlh->nlmsg_len = NLMSG_LENGTH(GENL_HDRLEN); nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK; @@ -253,6 +265,8 @@ int ipoe_nl_create() return -1; } + memset(&req, 0, sizeof(req)); + nlh = &req.n; nlh->nlmsg_len = NLMSG_LENGTH(GENL_HDRLEN); nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK; @@ -317,6 +331,8 @@ int ipoe_nl_modify(int ifindex, uint32_t peer_addr, uint32_t addr, uint32_t gw, return -1; } + memset(&req, 0, sizeof(req)); + nlh = &req.n; nlh->nlmsg_len = NLMSG_LENGTH(GENL_HDRLEN); nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK; @@ -406,6 +422,8 @@ void ipoe_nl_get_sessions(struct list_head *list) if (rth.fd == -1) return; + memset(&req, 0, sizeof(req)); + nlh = &req.n; nlh->nlmsg_len = NLMSG_LENGTH(GENL_HDRLEN); nlh->nlmsg_flags = NLM_F_ROOT | NLM_F_MATCH | NLM_F_REQUEST; @@ -438,6 +456,8 @@ void ipoe_nl_delete(int ifindex) return; } + memset(&req, 0, sizeof(req)); + nlh = &req.n; nlh->nlmsg_len = NLMSG_LENGTH(GENL_HDRLEN); nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK; -- cgit v1.2.3 From b2eab395bb283b412e7bc8fef022ac06721b9a4e Mon Sep 17 00:00:00 2001 From: Denys Fedoryshchenko Date: Sat, 8 Aug 2026 17:46:14 +0300 Subject: libnetlink: report the genl family id even when the group is not found genl_resolve_mcg() stored the resolved family id only after it had established that the family advertises multicast groups, so a caller that also needs the family id was left with nothing whenever the group lookup failed. Fill in fam_id as soon as it has been parsed. The return value is unchanged, so callers interested only in the group are unaffected. --- accel-pppd/libnetlink/genl.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'accel-pppd') diff --git a/accel-pppd/libnetlink/genl.c b/accel-pppd/libnetlink/genl.c index 0c758077..8cdfce89 100644 --- a/accel-pppd/libnetlink/genl.c +++ b/accel-pppd/libnetlink/genl.c @@ -82,12 +82,14 @@ int __export genl_resolve_mcg(const char *family, const char *name, int *fam_id) goto out; } - if (!tb[CTRL_ATTR_MCAST_GROUPS]) - goto out; - + /* report the family id even if the group lookup below fails, the + * caller may still have use for it */ if (fam_id) *fam_id = *(uint16_t *)(RTA_DATA(tb[CTRL_ATTR_FAMILY_ID])); + if (!tb[CTRL_ATTR_MCAST_GROUPS]) + goto out; + parse_rtattr_nested(tb2, GENL_MAX_FAM_GRPS, tb[CTRL_ATTR_MCAST_GROUPS]); for (i = 1; i < GENL_MAX_FAM_GRPS; i++) { -- cgit v1.2.3 From 6693d18ae0232da2c6febc0decf099e4fd064381 Mon Sep 17 00:00:00 2001 From: Denys Fedoryshchenko Date: Sat, 8 Aug 2026 17:47:49 +0300 Subject: ipoe: flush sessions left by a previous instance with a single command On startup accel-pppd is expected to drop everything a previous instance left behind in the kernel. For sessions it did so by dumping them with IPOE_CMD_GET and sending one IPOE_CMD_DELETE per ifindex. That dump was written for the session backup code removed in 1972a7e5c and was never adapted to its new, destructive role: - it was issued on the socket subscribed to the packet multicast group, so it competed with the notifications that the still attached stale rx handlers keep generating; on a loaded box the receive buffer overruns and rtnl_dump_filter() gives up with ENOBUFS, - its return value was discarded, so such a failure was silent, - it ran before the interfaces were detached, which is what produces those notifications in the first place, - it was skipped entirely when the multicast group could not be resolved, again with nothing but a warning about packet handling, - and every session cost a socket, a round trip, a grace period and a full unregister_netdev(). Whatever it missed stays in the kernel forever: the daemon has no record of those sessions, so nothing ever deletes them, and every subscriber later assigned one of their addresses is refused with EEXIST by IPOE_CMD_MODIFY. Add IPOE_CMD_FLUSH, which unlinks all sessions in one go, waits for a single grace period and unregisters the devices with unregister_netdevice_many(), and use it instead. The old path is kept as a fallback for a module predating the command, which is reported as EOPNOTSUPP, and now checks its return value and uses a private socket. Reorder init() so the interfaces are detached before the multicast group is joined, and so the flush also runs when only the group lookup failed. --- accel-pppd/ctrl/ipoe/ipoe.h | 3 +- accel-pppd/ctrl/ipoe/ipoe_netlink.c | 101 +++++++++++++++++++++++++++++++----- drivers/ipoe/ipoe.c | 54 +++++++++++++++++++ drivers/ipoe/ipoe.h | 1 + 4 files changed, 146 insertions(+), 13 deletions(-) (limited to 'accel-pppd') diff --git a/accel-pppd/ctrl/ipoe/ipoe.h b/accel-pppd/ctrl/ipoe/ipoe.h index 37c26a63..2bd3a14f 100644 --- a/accel-pppd/ctrl/ipoe/ipoe.h +++ b/accel-pppd/ctrl/ipoe/ipoe.h @@ -164,7 +164,8 @@ void ipoe_nl_delete_interfaces(void); int ipoe_nl_create(); void ipoe_nl_delete(int ifindex); int ipoe_nl_modify(int ifindex, uint32_t peer_addr, uint32_t addr, uint32_t gw, int link_ifindex, uint8_t *hwaddr); -void ipoe_nl_get_sessions(struct list_head *list); +int ipoe_nl_get_sessions(struct list_head *list); +int ipoe_nl_flush_sessions(void); int ipoe_nl_add_exclude(uint32_t addr, int mask); void ipoe_nl_del_exclude(uint32_t addr); int ipoe_nl_add_net(uint32_t addr, int mask); diff --git a/accel-pppd/ctrl/ipoe/ipoe_netlink.c b/accel-pppd/ctrl/ipoe/ipoe_netlink.c index 94b7f42a..6eee2dca 100644 --- a/accel-pppd/ctrl/ipoe/ipoe_netlink.c +++ b/accel-pppd/ctrl/ipoe/ipoe_netlink.c @@ -410,17 +410,23 @@ static int dump_session(const struct sockaddr_nl *addr, struct nlmsghdr *n, void return 0; } -void ipoe_nl_get_sessions(struct list_head *list) +int ipoe_nl_get_sessions(struct list_head *list) { + struct rtnl_handle rth; struct nlmsghdr *nlh; struct genlmsghdr *ghdr; struct { struct nlmsghdr n; char buf[1024]; } req; + int ret; - if (rth.fd == -1) - return; + /* a private socket, so that the dump does not have to compete with + * the packet notifications delivered to the multicast one */ + if (rtnl_open_byproto(&rth, 0, NETLINK_GENERIC)) { + log_error("ipoe: cannot open generic netlink socket\n"); + return -1; + } memset(&req, 0, sizeof(req)); @@ -434,11 +440,51 @@ void ipoe_nl_get_sessions(struct list_head *list) ghdr->cmd = IPOE_CMD_GET; if (rtnl_send(&rth, (char *)nlh, nlh->nlmsg_len) < 0) { - log_emerg("ipoe: failed to send dump request: %s\n", strerror(errno)); - return; + log_error("ipoe: failed to send dump request: %s\n", strerror(errno)); + rtnl_close(&rth); + return -1; + } + + ret = rtnl_dump_filter(&rth, dump_session, list, NULL, NULL); + + rtnl_close(&rth); + + return ret; +} + +int ipoe_nl_flush_sessions(void) +{ + struct rtnl_handle rth; + struct nlmsghdr *nlh; + struct genlmsghdr *ghdr; + struct { + struct nlmsghdr n; + char buf[128]; + } req; + int ret = 0; + + if (rtnl_open_byproto(&rth, 0, NETLINK_GENERIC)) { + log_error("ipoe: cannot open generic netlink socket\n"); + errno = EIO; + return -1; } - rtnl_dump_filter(&rth, dump_session, list, NULL, NULL); + memset(&req, 0, sizeof(req)); + + nlh = &req.n; + nlh->nlmsg_len = NLMSG_LENGTH(GENL_HDRLEN); + nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK; + nlh->nlmsg_type = ipoe_genl_id; + + ghdr = NLMSG_DATA(&req.n); + ghdr->cmd = IPOE_CMD_FLUSH; + + if (rtnl_talk(&rth, nlh, 0, 0, nlh, NULL, NULL, 0) < 0) + ret = -1; + + rtnl_close(&rth); + + return ret; } void ipoe_nl_delete(int ifindex) @@ -479,7 +525,10 @@ static void delete_sessions() struct ipoe_session_info *info; LIST_HEAD(ds_list); - ipoe_nl_get_sessions(&ds_list); + + if (ipoe_nl_get_sessions(&ds_list)) + log_error("ipoe: failed to enumerate sessions left by a previous" + " instance, some of them are not removed\n"); while (!list_empty(&ds_list)) { info = list_entry(ds_list.next, typeof(*info), entry); @@ -489,6 +538,22 @@ static void delete_sessions() } } +static void flush_sessions() +{ + if (!ipoe_nl_flush_sessions()) + return; + + if (errno == EOPNOTSUPP) { + log_warn("ipoe: loaded ipoe module does not support IPOE_CMD_FLUSH," + " removing sessions one by one, reload the module to fix\n"); + delete_sessions(); + return; + } + + log_error("ipoe: failed to remove sessions left by a previous instance:" + " %s\n", strerror(errno)); +} + static void ipoe_up_handler(const struct sockaddr_nl *addr, struct nlmsghdr *h) { struct rtattr *tb[PKT_ATTR_MAX + 1]; @@ -648,6 +713,23 @@ static void init(void) log_warn("failed to load ipoe module\n"); mcg_id = genl_resolve_mcg(IPOE_GENL_NAME, IPOE_GENL_MCG_PKT, &ipoe_genl_id); + + if (!ipoe_genl_id) { + log_error("ipoe: cannot resolve netlink family, state left by a" + " previous instance is not removed\n"); + rth.fd = -1; + return; + } + + /* Drop everything a previous instance may have left in the kernel. + * The interfaces go first: while their rx handlers are still attached + * the module keeps reporting unclassified packets, and once we join + * the multicast group that traffic competes with our own replies. */ + ipoe_nl_delete_interfaces(); + flush_sessions(); + ipoe_nl_del_exclude(0); + ipoe_nl_del_net(0); + if (mcg_id == -1) { log_warn("ipoe: unclassified packet handling is disabled\n"); rth.fd = -1; @@ -660,11 +742,6 @@ static void init(void) return; } - delete_sessions(); - ipoe_nl_del_exclude(0); - ipoe_nl_del_net(0); - ipoe_nl_delete_interfaces(); - fcntl(rth.fd, F_SETFL, O_NONBLOCK); fcntl(rth.fd, F_SETFD, fcntl(rth.fd, F_GETFD) | FD_CLOEXEC); diff --git a/drivers/ipoe/ipoe.c b/drivers/ipoe/ipoe.c index 73926994..8f1c852b 100644 --- a/drivers/ipoe/ipoe.c +++ b/drivers/ipoe/ipoe.c @@ -1420,6 +1420,52 @@ out_unlock: return ret; } +static int ipoe_nl_cmd_flush(struct sk_buff *skb, struct genl_info *info) +{ + struct ipoe_session *ses; + LIST_HEAD(list); + LIST_HEAD(kill_list); + + down(&ipoe_wlock); + + list_splice_init(&ipoe_list2, &list); + + list_for_each_entry(ses, &list, entry2) { + if (ses->peer_addr) + list_del_rcu(&ses->entry); + if (ses->u.hwaddr_u) + list_del_rcu(&ses->entry3); + } + + up(&ipoe_wlock); + + if (list_empty(&list)) + return 0; + + /* a single grace period covers the whole batch */ + synchronize_rcu(); + + list_for_each_entry(ses, &list, entry2) { + while (atomic_read(&ses->refs)) + schedule_timeout_uninterruptible(1); + + if (ses->link_dev) { + dev_put(ses->link_dev); + ses->link_dev = NULL; + } + } + + rtnl_lock(); + list_for_each_entry(ses, &list, entry2) + unregister_netdevice_queue(ses->dev, &kill_list); + unregister_netdevice_many(&kill_list); + rtnl_unlock(); + + /* the sessions are freed by now, do not touch 'list' again */ + + return 0; +} + static int ipoe_nl_cmd_modify(struct sk_buff *skb, struct genl_info *info) { int ret = -EINVAL, r = 0; @@ -1894,6 +1940,14 @@ static const struct genl_ops ipoe_nl_ops[] = { .flags = GENL_ADMIN_PERM, #if LINUX_VERSION_CODE < KERNEL_VERSION(5,2,0) .policy = ipoe_nl_policy, +#endif + }, + { + .cmd = IPOE_CMD_FLUSH, + .doit = ipoe_nl_cmd_flush, + .flags = GENL_ADMIN_PERM, +#if LINUX_VERSION_CODE < KERNEL_VERSION(5,2,0) + .policy = ipoe_nl_policy, #endif }, }; diff --git a/drivers/ipoe/ipoe.h b/drivers/ipoe/ipoe.h index 4097e2da..2d041c50 100644 --- a/drivers/ipoe/ipoe.h +++ b/drivers/ipoe/ipoe.h @@ -16,6 +16,7 @@ enum { IPOE_CMD_DEL_EXCLUDE, IPOE_CMD_ADD_NET, IPOE_CMD_DEL_NET, + IPOE_CMD_FLUSH, __IPOE_CMD_MAX, }; -- cgit v1.2.3 From fc465210fb77af7d6a12780f3f256d17be2ce480 Mon Sep 17 00:00:00 2001 From: Denys Fedoryshchenko Date: Sat, 8 Aug 2026 23:17:46 +0300 Subject: fixup! ipoe: flush sessions left by a previous instance with a single command --- accel-pppd/ctrl/ipoe/ipoe_netlink.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) (limited to 'accel-pppd') diff --git a/accel-pppd/ctrl/ipoe/ipoe_netlink.c b/accel-pppd/ctrl/ipoe/ipoe_netlink.c index 6eee2dca..d611c5f9 100644 --- a/accel-pppd/ctrl/ipoe/ipoe_netlink.c +++ b/accel-pppd/ctrl/ipoe/ipoe_netlink.c @@ -465,8 +465,7 @@ int ipoe_nl_flush_sessions(void) if (rtnl_open_byproto(&rth, 0, NETLINK_GENERIC)) { log_error("ipoe: cannot open generic netlink socket\n"); - errno = EIO; - return -1; + return -EIO; } memset(&req, 0, sizeof(req)); @@ -480,7 +479,7 @@ int ipoe_nl_flush_sessions(void) ghdr->cmd = IPOE_CMD_FLUSH; if (rtnl_talk(&rth, nlh, 0, 0, nlh, NULL, NULL, 0) < 0) - ret = -1; + ret = errno ? -errno : -EIO; rtnl_close(&rth); @@ -540,10 +539,12 @@ static void delete_sessions() static void flush_sessions() { - if (!ipoe_nl_flush_sessions()) + int r = ipoe_nl_flush_sessions(); + + if (!r) return; - if (errno == EOPNOTSUPP) { + if (r == -EOPNOTSUPP) { log_warn("ipoe: loaded ipoe module does not support IPOE_CMD_FLUSH," " removing sessions one by one, reload the module to fix\n"); delete_sessions(); @@ -551,7 +552,7 @@ static void flush_sessions() } log_error("ipoe: failed to remove sessions left by a previous instance:" - " %s\n", strerror(errno)); + " %s\n", strerror(-r)); } static void ipoe_up_handler(const struct sockaddr_nl *addr, struct nlmsghdr *h) -- cgit v1.2.3 From 811f87f071659410f9126d51b6ef72a4c64a02a9 Mon Sep 17 00:00:00 2001 From: Denys Fedoryshchenko Date: Sat, 8 Aug 2026 23:57:09 +0300 Subject: fixup! ipoe: flush sessions left by a previous instance with a single command --- accel-pppd/ctrl/ipoe/ipoe_netlink.c | 3 --- 1 file changed, 3 deletions(-) (limited to 'accel-pppd') diff --git a/accel-pppd/ctrl/ipoe/ipoe_netlink.c b/accel-pppd/ctrl/ipoe/ipoe_netlink.c index d611c5f9..e0f41cdf 100644 --- a/accel-pppd/ctrl/ipoe/ipoe_netlink.c +++ b/accel-pppd/ctrl/ipoe/ipoe_netlink.c @@ -718,7 +718,6 @@ static void init(void) if (!ipoe_genl_id) { log_error("ipoe: cannot resolve netlink family, state left by a" " previous instance is not removed\n"); - rth.fd = -1; return; } @@ -733,13 +732,11 @@ static void init(void) if (mcg_id == -1) { log_warn("ipoe: unclassified packet handling is disabled\n"); - rth.fd = -1; return; } if (rtnl_open_byproto(&rth, 1 << (mcg_id - 1), NETLINK_GENERIC)) { log_error("ipoe: cannot open generic netlink socket\n"); - rth.fd = -1; return; } -- cgit v1.2.3 From 2b46f1c69ca77efc317673b080d4773db09a4a3e Mon Sep 17 00:00:00 2001 From: Denys Fedoryshchenko Date: Sun, 9 Aug 2026 00:18:35 +0300 Subject: fixup! ipoe: flush sessions left by a previous instance with a single command --- accel-pppd/ctrl/ipoe/ipoe_netlink.c | 1 + 1 file changed, 1 insertion(+) (limited to 'accel-pppd') diff --git a/accel-pppd/ctrl/ipoe/ipoe_netlink.c b/accel-pppd/ctrl/ipoe/ipoe_netlink.c index e0f41cdf..df4dec67 100644 --- a/accel-pppd/ctrl/ipoe/ipoe_netlink.c +++ b/accel-pppd/ctrl/ipoe/ipoe_netlink.c @@ -478,6 +478,7 @@ int ipoe_nl_flush_sessions(void) ghdr = NLMSG_DATA(&req.n); ghdr->cmd = IPOE_CMD_FLUSH; + errno = 0; if (rtnl_talk(&rth, nlh, 0, 0, nlh, NULL, NULL, 0) < 0) ret = errno ? -errno : -EIO; -- cgit v1.2.3