| Age | Commit message (Collapse) | Author |
|---|
|
struct rtnl_newlink_params was introduced in 6.15, but the newlink
callback switched to it at 6.13, so the module failed to build on
6.13 and 6.14 kernels with an incompatible pointer type for .newlink.
Signed-off-by: Denys Fedoryshchenko <denys.f@collabora.com>
|
|
Stable kernels backport the netns_local -> netns_immutable rename
(6.12.114+, e.g. Debian 13's 6.12.115), so LINUX_VERSION_CODE can no
longer tell which member the headers provide and the module fails to
build with "'struct net_device' has no member named 'netns_local'".
Grep the kernel's include/linux/netdevice.h from the Kbuild Makefile
and pass -DHAVE_NETNS_IMMUTABLE or -DHAVE_NETNS_LOCAL accordingly. The
version checks stay only as a fallback if the probe finds neither.
Signed-off-by: Denys Fedoryshchenko <denys.f@collabora.com>
|
|
rtnl_link_register() and struct rtnl_link_ops were reaching the driver
through some other header rather than through net/rtnetlink.h, which is
the kind of thing that only shows up when building against a different
kernel. Include it directly.
The workflows insert the kernel modules between two pytest runs, and the
runs that follow are marked 'if: always()' while the insmod steps are
not. A failure in an earlier, unrelated test therefore skips the insmod
but still runs the tests that need the module, which then report a
missing driver instead of the original problem. Mark the insmod steps
'if: always()' as well, so that the later runs test what they are
supposed to.
|
|
Sessions left in the kernel by a dead daemon could only be got rid of by
restarting accel-pppd, which flushes all of them and so drops everyone
still online, or by unloading the module. Neither is of much use when a
single stale interface is holding an address and every subscriber that
is later handed it is refused with EEXIST.
Register rtnl_link_ops, so that RTM_DELLINK reaches the driver:
ip link show type ipoe enumerate them
ip -d link show ipoe42 reports kind "ipoe"
ip link del ipoe42 remove a single one
newlink returns EOPNOTSUPP rather than being left out. A session carries
private state that IPOE_CMD_CREATE sets up, and a kernel that falls back
to register_netdevice() when newlink is NULL would hand out a device
with zeroed private state and no percpu counters, which oopses on the
first packet or on ip -s link show.
dellink runs under rtnl and outside genl_mutex, and is therefore the
first writer in this driver that does not serialise against the genl
handlers: it can unlink a session that IPOE_CMD_DELETE is about to
unlink as well, since the device stays registered until both are done.
Add ipoe_session.dying, set under ipoe_wlock by whichever path starts
the teardown, and let the other one bail out.
That leaves rtnl nested inside ipoe_wlock, which is safe because no path
does it the other way round: ipoe_create() releases rtnl before taking
ipoe_wlock, ipoe_nl_cmd_delete() releases ipoe_wlock before calling
unregister_netdev(), and the interface commands use rtnl alone. dellink
does have to sleep in synchronize_rcu() and while draining the session
refcount with rtnl held, which the genl path avoids by deferring both
until after it has dropped everything.
A session left behind by a dead daemon is indistinguishable from one
that is still in use - same peer address, same lists, same flags - so
the removal cannot be refused on state alone. Warn instead, and only
when somebody is still subscribed to the packet multicast group, so that
clearing stale interfaces on a box where accel-pppd is not running stays
quiet.
|
|
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.
|
|
ipoe_nl_cmd_dump_sessions() increments idx before calling fill_info(),
so once fill_info() fails because the skb is full, idx already points
past the session that did not fit. cb->args[0] is set to that value and
the next round resumes one entry too far, dropping the session from the
dump entirely - one lost session per message boundary, roughly one in
every 90 at the current record size.
Step idx back before leaving the loop.
|
|
ipoe_nl_cmd_delete() drops ipoe_wlock before sleeping in
synchronize_rcu() and taking rtnl via unregister_netdev(), then falls
through into the out_unlock label and releases it a second time. Every
successful session delete therefore increments the semaphore count by
one.
Since the count only ever grows, ipoe_wlock stops providing mutual
exclusion after a handful of teardowns: with the count at N, up to N+1
writers may hold it simultaneously. This is currently masked because
ipoe_nl_family does not set parallel_ops, so genetlink serialises every
.doit/.dumpit under genl_mutex and no two writers can overlap in
practice. It turns into a real race on the session hash lists as soon as
that changes, or as soon as a writer is introduced outside the genl
handlers.
It also silently defeats the barrier in ipoe_fini(): the down()/up()
pair meant to wait for an in-flight writer is satisfied immediately by
the leaked count and waits for nothing.
Return directly after unregister_netdev() so the success path releases
the lock exactly once. The early up() is deliberate and stays where it
is - holding a sleeping semaphore across unregister_netdev() would nest
ipoe_wlock inside rtnl, while ipoe_create() takes rtnl first and
ipoe_wlock afterwards. Error paths, return values and lock ordering are
unchanged.
|
|
ipoe_recv() handles every ETH_P_IPV6 frame arriving on an IPoE
by calling ipoe_lookup_rt6()/ip6_route_output().
On ipv6.disable=1 IPv6 FIB is not initialized.
We can detect that by ipv6_mod_enabled() since kernel 4.8,
and return RX_HANDLER_PASS, so packet falls thru normal stack and get discarded.
Fixes: https://github.com/accel-ppp/accel-ppp/issues/313
Signed-off-by: Denys Fedoryshchenko <denys.f@collabora.com>
|
|
Part of a long-term kernel networking cleanup is the kernel is moving to a strict type called dscp_t.
1)Macros like flowi4_tos are being removed to break compilation of old drivers (like accel-ppp) that treat the field as a raw byte.
2)This forces developers to use the new accessor functions (like ip4_dst_hoplimit or inet_dscp_to_dsfield) ensuring ECN bits are preserved.
We need to maintain compatibility with older kernel as well.
Signed-off-by: Denys Fedoryshchenko <denys.f@collabora.com>
|
|
NETIF_F_NETNS_LOCAL existed in <=6.11, then converted to
dev->netns_local, and then renamed to netns_immutable in
0c493da86374dffff7505e67289ad75b21f5b301.
This commit address this properly.
Signed-off-by: Denys Fedoryshchenko <denys.f@collabora.com>
|
|
del_timer() was renamed to timer_delete() by commit bb663f0f3c396c6d
(“timers: Rename del_timer() to timer_delete()”) authored by Thomas Gleixner
on 23 Nov 2022, merged via the timers/core branch and included in the Linux
6.2-rc1 merge window; the change therefore reached users with the Linux 6.2
final release on 19 Feb 2023.
(The legacy inline wrappers del_timer() / del_timer_sync() stayed in place
for compatibility until they were dropped by the tree-wide cleanup commit
8fa7292fee5c on 5 Apr 2025, merged for Linux 6.15-rc1.)
Signed-off-by: Denys Fedoryshchenko <denys.f@collabora.com>
|
|
Closes: https://github.com/accel-ppp/accel-ppp/issues/217
Ref: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=05c1280a2bcfca187fe7fa90bb240602cf54af0a
Ref: https://github.com/torvalds/linux/commit/05c1280a2bcfca187fe7fa90bb240602cf54af0a
Reported-By: https://github.com/axe-kenig
|
|
|
|
Signed-off-by: Sergey V. Lobanov <sergey@lobanov.in>
|
|
Signed-off-by: Denys Fedoryshchenko <denys.f@collabora.com>
|
|
|
|
|
|
Co-authored-by: Martin Zaharinov <micron@bngsoft.net>
|
|
|
|
* Fix errors found by valgrind
==12312== Syscall param sendmsg(msg.msg_iov[0]) points to uninitialised byte(s)
==12312== at 0x486CCF0: sendmsg (in /lib/libpthread-2.22.so)
==12312== by 0x12F57F: rtnl_talk (libnetlink.c:316)
==12312== by 0x132DA3: genl_resolve_mcg (genl.c:52)
==12312== by 0x484E1CB: init (vlan_mon.c:528)
==12312== by 0x484CDC0: vlan_mon_register_proto (vlan_mon.c:48)
==12312== by 0x510B763: load_vlan_mon (pppoe.c:1914)
==12312== by 0x510BFF2: load_config (pppoe.c:2064)
==12312== by 0x510C22A: pppoe_init (pppoe.c:2108)
==12312== by 0x483E9EB: triton_load_modules (triton.c:704)
==12312== by 0x1384B2: main (main.c:339)
==12312== Address 0xbedacdd8 is on thread 1's stack
==12312== in frame #2, created by genl_resolve_mcg (genl.c:23)
==12312== 15 bytes in 1 blocks are definitely lost in loss record 352 of 836
==12312== at 0x482A9A9: malloc (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==12312== by 0x4B97524: strdup (in /lib/libc-2.22.so)
==12312== by 0x12C30C: init (telnet.c:769)
==12312== by 0x483E9EB: triton_load_modules (triton.c:704)
==12312== by 0x1384B2: main (main.c:339)
==12312==
==12312== 15 bytes in 1 blocks are definitely lost in loss record 353 of 836
==12312== at 0x482A9A9: malloc (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==12312== by 0x4B97524: strdup (in /lib/libc-2.22.so)
==12312== by 0x12D60A: init (tcp.c:392)
==12312== by 0x483E9EB: triton_load_modules (triton.c:704)
==12312== by 0x1384B2: main (main.c:339)
* Fix another warnings by cppcheck
[accel-pppd/ctrl/ipoe/arp.c:256]: (error) Uninitialized variable: n
[accel-pppd/ctrl/pppoe/mac_filter.c:90]: (warning) %x in format string (no. 1) requires 'unsigned int *' but the argument type is 'signed int *'.
[accel-pppd/ctrl/pppoe/mac_filter.c:90]: (warning) %x in format string (no. 2) requires 'unsigned int *' but the argument type is 'signed int *'.
[accel-pppd/ctrl/pppoe/mac_filter.c:90]: (warning) %x in format string (no. 3) requires 'unsigned int *' but the argument type is 'signed int *'.
[accel-pppd/ctrl/pppoe/mac_filter.c:90]: (warning) %x in format string (no. 4) requires 'unsigned int *' but the argument type is 'signed int *'.
[accel-pppd/ctrl/pppoe/mac_filter.c:90]: (warning) %x in format string (no. 5) requires 'unsigned int *' but the argument type is 'signed int *'.
[accel-pppd/ctrl/pppoe/mac_filter.c:90]: (warning) %x in format string (no. 6) requires 'unsigned int *' but the argument type is 'signed int *'.
[accel-pppd/ctrl/pppoe/mac_filter.c:129]: (warning) %x in format string (no. 1) requires 'unsigned int *' but the argument type is 'signed int *'.
[accel-pppd/ctrl/pppoe/mac_filter.c:129]: (warning) %x in format string (no. 2) requires 'unsigned int *' but the argument type is 'signed int *'.
[accel-pppd/ctrl/pppoe/mac_filter.c:129]: (warning) %x in format string (no. 3) requires 'unsigned int *' but the argument type is 'signed int *'.
[accel-pppd/ctrl/pppoe/mac_filter.c:129]: (warning) %x in format string (no. 4) requires 'unsigned int *' but the argument type is 'signed int *'.
[accel-pppd/ctrl/pppoe/mac_filter.c:129]: (warning) %x in format string (no. 5) requires 'unsigned int *' but the argument type is 'signed int *'.
[accel-pppd/ctrl/pppoe/mac_filter.c:129]: (warning) %x in format string (no. 6) requires 'unsigned int *' but the argument type is 'signed int *'.
[accel-pppd/ctrl/pppoe/mac_filter.c:158]: (warning) %x in format string (no. 1) requires 'unsigned int *' but the argument type is 'signed int *'.
[accel-pppd/ctrl/pppoe/mac_filter.c:158]: (warning) %x in format string (no. 2) requires 'unsigned int *' but the argument type is 'signed int *'.
[accel-pppd/ctrl/pppoe/mac_filter.c:158]: (warning) %x in format string (no. 3) requires 'unsigned int *' but the argument type is 'signed int *'.
[accel-pppd/ctrl/pppoe/mac_filter.c:158]: (warning) %x in format string (no. 4) requires 'unsigned int *' but the argument type is 'signed int *'.
[accel-pppd/ctrl/pppoe/mac_filter.c:158]: (warning) %x in format string (no. 5) requires 'unsigned int *' but the argument type is 'signed int *'.
[accel-pppd/ctrl/pppoe/mac_filter.c:158]: (warning) %x in format string (no. 6) requires 'unsigned int *' but the argument type is 'signed int *'.
[accel-pppd/ctrl/pppoe/pppoe.c:738]: (warning) Possible null pointer dereference
[accel-pppd/ipv6/dhcpv6.c:911]: (warning) %x in format string (no. 1) requires 'unsigned int *' but the argument type is 'int *'.
[accel-pppd/ipv6/dhcpv6.c:911]: (warning) %x in format string (no. 2) requires 'unsigned int *' but the argument type is 'signed int *'.
[accel-pppd/ipv6/dhcpv6.c:911]: (warning) %x in format string (no. 3) requires 'unsigned int *' but the argument type is 'signed int *'.
[accel-pppd/ipv6/dhcpv6.c:911]: (warning) %x in format string (no. 4) requires 'unsigned int *' but the argument type is 'signed int *'.
[accel-pppd/libnetlink/libnetlink.c:515]: (warning) Possible null pointer dereference
[accel-pppd/ppp/ipv6cp_opt_intfid.c:185]: (warning) %x in format string (no. 1) requires 'unsigned int *' but the argument type is 'signed int *'.
[accel-pppd/ppp/ipv6cp_opt_intfid.c:298]: (warning) %x in format string (no. 1) requires 'unsigned int *' but the argument type is 'int *'.
[accel-pppd/ppp/ipv6cp_opt_intfid.c:298]: (warning) %x in format string (no. 2) requires 'unsigned int *' but the argument type is 'signed int *'.
[accel-pppd/ppp/ipv6cp_opt_intfid.c:298]: (warning) %x in format string (no. 3) requires 'unsigned int *' but the argument type is 'signed int *'.
[accel-pppd/ppp/ipv6cp_opt_intfid.c:298]: (warning) %x in format string (no. 4) requires 'unsigned int *' but the argument type is 'signed int *'.
* Suppress compiler warnings
* Fix locking errors
/opt/pvs/accel-ppp/accel-pppd/extra/pppd_compat.c 279 warn V1020 The function exited without calling the 'sigchld_unlock' function. Check lines: 279, 249.
/opt/pvs/accel-ppp/accel-pppd/extra/pppd_compat.c 333 warn V1020 The function exited without calling the 'sigchld_unlock' function. Check lines: 333, 315.
/opt/pvs/accel-ppp/accel-pppd/extra/pppd_compat.c 422 warn V1020 The function exited without calling the 'sigchld_unlock' function. Check lines: 422, 372.
/opt/pvs/accel-ppp/accel-pppd/extra/pppd_compat.c 488 warn V1020 The function exited without calling the 'sigchld_unlock' function. Check lines: 488, 468.
/opt/pvs/accel-ppp/accel-pppd/triton/mempool.c 119 warn V1020 The function exited without calling the 'pthread_spin_unlock' function. Check lines: 119, 116.
* Fix array len errors
/opt/pvs/accel-ppp/accel-pppd/triton/conf_file.c 75 warn V557 Array underrun is possible. The value of 'len - 1' index could reach -1.
/opt/pvs/accel-ppp/accel-pppd/triton/conf_file.c 76 warn V557 Array underrun is possible. The value of '-- len' index could reach -1.
* Fix possible memory leaks
/opt/pvs/accel-ppp/accel-pppd/radius/radius.c 936 err V773 The function was exited without releasing the 'str' pointer. A memory leak is possible.
/opt/pvs/accel-ppp/accel-pppd/radius/serv.c 622 err V773 The function was exited without releasing the 'str' pointer. A memory leak is possible.
/opt/pvs/accel-ppp/accel-pppd/triton/conf_file.c 144 err V773 The function was exited without releasing the 'raw' pointer. A memory leak is possible.
* Fix unsafe code
/opt/pvs/accel-ppp/accel-pppd/cli/tcp.c 364 warn V1004 The 'host' pointer was used unsafely after it was verified against nullptr. Check lines: 338, 364.
/opt/pvs/accel-ppp/accel-pppd/cli/telnet.c 701 warn V1004 The 'host' pointer was used unsafely after it was verified against nullptr. Check lines: 675, 701.
/opt/pvs/accel-ppp/accel-pppd/extra/ippool.c 241 err V614 Potentially uninitialized pointer 'pos' used.
/opt/pvs/accel-ppp/accel-pppd/radius/dict.c 165 err V614 Uninitialized pointer 'parent_items' used.
* Remove duplicate code
/opt/pvs/accel-ppp/accel-pppd/radius/serv.c 202 warn V547 Expression 'ts.tv_sec < req->serv->fail_time' is always false.
* Fix treating signed bool variables as unsigned
* Add nullptr checking
/opt/pvs/accel-ppp/accel-pppd/ipv6/dhcpv6.c 886 err V595 The 'opt->val' pointer was utilized before it was verified against nullptr. Check lines: 886, 890.
/opt/pvs/accel-ppp/accel-pppd/ipv6/nd.c 479 err V595 The 'opt->val' pointer was utilized before it was verified against nullptr. Check lines: 479, 483.
/opt/pvs/accel-ppp/accel-pppd/radius/auth.c 152 err V595 The 'rpd->auth_ctx' pointer was utilized before it was verified against nullptr. Check lines: 152, 154.
/opt/pvs/accel-ppp/accel-pppd/triton/conf_file.c 114 err V595 The 'cur_sect' pointer was utilized before it was verified against nullptr. Check lines: 114, 117.
* Add logging of exit conditions
* Clarify calculation
[accel-pppd/ppp/ccp_mppe.c:281]: (style) Clarify calculation precedence for '&' and '?'.
[accel-pppd/ppp/ccp_mppe.c:282]: (style) Clarify calculation precedence for '&' and '?'.
[accel-pppd/ppp/ccp_mppe.c:283]: (style) Clarify calculation precedence for '&' and '?'.
[accel-pppd/ppp/ccp_mppe.c:284]: (style) Clarify calculation precedence for '&' and '?'.
[accel-pppd/ppp/ccp_mppe.c:285]: (style) Clarify calculation precedence for '&' and '?'.
[accel-pppd/ppp/ccp_mppe.c:286]: (style) Clarify calculation precedence for '&' and '?'.
[drivers/ipoe/ipoe.c:307]: (style) Clarify calculation precedence for '&' and '?'.
* Fix void calculations
[accel-pppd/ctrl/pppoe/disc.c:211]: (portability) 'pkt' is of type 'void *'. When using void pointers in calculations, the behaviour is undefined.
[accel-pppd/ctrl/pptp/pptp.c:150]: (portability) 'buf' is of type 'void *'. When using void pointers in calculations, the behaviour is undefined.
[accel-pppd/radius/acct.c:37]: (portability) 'req.pack.buf' is of type 'void *'. When using void pointers in calculations, the behaviour is undefined.
[accel-pppd/radius/auth.c:35]: (portability) 'req.pack.buf' is of type 'void *'. When using void pointers in calculations, the behaviour is undefined.
[accel-pppd/radius/auth.c:79]: (portability) 'req.pack.buf' is of type 'void *'. When using void pointers in calculations, the behaviour is undefined.
[accel-pppd/radius/dm_coa.c:43]: (portability) 'pack.buf' is of type 'void *'. When using void pointers in calculations, the behaviour is undefined.
[accel-pppd/radius/dm_coa.c:47]: (portability) 'pack.buf' is of type 'void *'. When using void pointers in calculations, the behaviour is undefined.
[accel-pppd/radius/dm_coa.c:57]: (portability) 'pack.buf' is of type 'void *'. When using void pointers in calculations, the behaviour is undefined.
[accel-pppd/radius/dm_coa.c:65]: (portability) 'req.buf' is of type 'void *'. When using void pointers in calculations, the behaviour is undefined.
[accel-pppd/radius/dm_coa.c:97]: (portability) 'req.buf' is of type 'void *'. When using void pointers in calculations, the behaviour is undefined.
[accel-pppd/radius/serv.c:364]: (portability) 'req.pack.buf' is of type 'void *'. When using void pointers in calculations, the behaviour is undefined.
[accel-pppd/triton/mempool.c:115]: (portability) 'mmap_ptr' is of type 'void *'. When using void pointers in calculations, the behaviour is undefined.
[accel-pppd/triton/mempool.c:122]: (portability) 'mmap_ptr' is of type 'void *'. When using void pointers in calculations, the behaviour is undefined.
[accel-pppd/triton/mempool.c:276]: (portability) 'ptr' is of type 'void *'. When using void pointers in calculations, the behaviour is undefined.
* Fix void part 2
[accel-pppd/ipv6/dhcpv6.c:844]: (portability) 'conf_dnssl' is of type 'void *'. When using void pointers in calculations, the behaviour is undefined.
[accel-pppd/ipv6/nd.c:199]: (portability) '(void*)dnsslinfo' is of type 'void *'. When using void pointers in calculations, the behaviour is undefined.
[accel-pppd/ipv6/nd.c:432]: (portability) 'conf_dnssl' is of type 'void *'. When using void pointers in calculations, the behaviour is undefined.
* Fix null pointer dereference
[accel-pppd/ctrl/ipoe/ipoe.c:2048]: (warning) Possible null pointer dereference: eth
[accel-pppd/ctrl/ipoe/ipoe.c:2049]: (warning) Possible null pointer dereference: iph
* Remove redundant check
/opt/pvs/accel-ppp/accel-pppd/ctrl/l2tp/packet.c 656 warn V547 Expression 'attr->length <= 16' is always false.
* PR fixes
* PR fixes 2
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
local-net is used to filter incomming packets which may start UP session
|
|
|
|
|
|
|
|
|
|
|
|
Virtual ipoe interfaces can not be used to attach wide routes, only routes behind client ip (gateway)
|
|
Makes possible to attach routes to virtual ipoe interfaces
|
