| Age | Commit message (Collapse) | Author |
|---|
|
cmake: fix kernel module installation
|
|
Install each module from its real build directory, propagate staged install roots, and fail when kbuild modules_install fails. Add a rootless CTest and CI regression for successful and failed installs.
Best practices followed:
Use the standard CMake DESTDIR and kbuild INSTALL_MOD_PATH staging interfaces:
https://cmake.org/cmake/help/latest/envvar/DESTDIR.html
https://docs.kernel.org/kbuild/modules.html#install-mod-path
Check child command status with execute_process RESULT_VARIABLE:
https://cmake.org/cmake/help/latest/command/execute_process.html
Register repeatable tests with CTest add_test:
https://cmake.org/cmake/help/latest/command/add_test.html
|
|
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>
|
|
It should look like:
[ 99%] Generating driver/ipoe.ko for kernel $(make -s -C ${KDIR}
kernelrelease 2>/dev/null || uname -r)" right before the module build
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)
|
