Bug#990739: buster-pu: package iptables-netflow/2.3-5+deb10u1
Package: release.debian.org
Severity: normal
Tags: buster
User: release.debian.org@packages.debian.org
Usertags: pu
X-Debbugs-Cc: abe@debian.org, carnil@debian.org
Hi,
an API change in the Linux kernel 4.19.194-1 uploaded with the Buster
10.10 stable minor update caused a regression in
iptables-netflow-dkms/2.3-5 built from the iptables-netflow source
package. The upstream API change happened in 4.19.191:
- modules: mark ref_module static
Relevant bug reports:
* Debian: https://bugs.debian.org/990123
* Upstream: https://github.com/aabc/ipt-netflow/issues/177
I would like to upload an updated package of iptables-netflow to
buster-proposed-updates which cherry-picks two upstream patches (see
below under [Changes] for details) which fix the issue initially and
then also for updated stable kernel lines like those in Buster.
[ Reason ]
Linux upstream has been backporting a change from kernel 5.9 to stable
kernel releases which makes sure that kernel modules which claim to be
GPL licensed and use _GPL exports, can no more depend on symbols from
non-GPL modules. This is has been solved by marking a function static,
i.e. no more being usable by kernel modules.
The Debian kernel team stated in that it's unlikely that Linux kernel
upstream will revert the patches and they also stated that it's
unlikely that Debian's linux kernel will divert from upstream at this
point.
Context about this issue:
https://lore.kernel.org/lkml/20200730061027.29472-1-hch@lst.de/
https://lore.kernel.org/stable/YMxnXQzcP0g1F9Iw@kroah.com/
(Thanks to Salvatore Bonaccorso of the Debian kernel team for these
links and further reviews and suggestions on this issue!)
[ Impact ]
The package is currently no more working after a reboot into a current
Buster 10.10 kernel as the DKMS kernel module fails to build with
current kernel headers (see #990123). It is currently still usable
with kernels before 4.19.194-1.
It will also no more compile with non-debian kernels of the stable
kernel lines 4.14 (version 4.14.233 and above) and 5.4 (version 5.4.11
and above). (Compilation of kernels above 5.9rc1 never worked with the
version in Buster.)
[ Tests ]
The .deb as generated when applying the debdiff below runs in
production for about 1.5 weeks on two of my netflow generating
servers, first with kernel version 4.19.194-1, later with kernel
version 4.19.194-2, both with ABI 4.19.0-17-amd64.
I also tried installing it (aka compiling the DKMS module) on a box
which was still running linux-image-4.19.0-12-amd64 (package version
4.19.152-1 + headers) from October 2020. Since also further Debian
kernels were installed, I also successfully tested its compilation
against linux-{image,headers}-4.19.0-14-amd64 (package version
4.19.171-2).
No issues have been observed so far. Functionality is as expected.
[ Risks and Expected Regressions ]
The upstream patch https://github.com/aabc/ipt-netflow/commit/352cdb28
mostly removes CPP "#if LINUX_VERSION_CODE >= KERNEL_VERSION(…)"
blocks containing legacy code not needed for more modern kernels and
enables the modern code also for older releases.
As I read that upstream commit, now this kernel module will no more
compile with (vanilla) kernels before 2.6.35 which seems to have
introduced the functionality which is now used instead of the function
made static in 5.9.0, 4.19.191 and other recent stable kernel
releases. (I though didn't test any other kernels than those in Debian
Buster. For older kernels than 4.19.194-1 I just tested if the DKMS
module still compiles, not if it still works as before.)
Since upstream's approach also compiled against older stable kernels
than those affected by #990123 I took upstream's approach instead of
making those "#if LINUX_VERSION_CODE >= KERNEL_VERSION(…)" checks even
more complex by adding further constraints to list all the updated
stable kernels mentioned above.
[ Checklist ]
[√] *all* changes are documented in the d/changelog
[√] I reviewed all changes and I approve them
[√] attach debdiff against the package in (old)stable
[*] the issue is verified as fixed in unstable
Footnotes: * = Patch 1 (cherry-picked adfc6318) is already included in
Debian Unstable and Bullseye as a cherry-picked patch
from the currently most recent upstream 2.6 release. It
fixes the same issue for kernels 5.9 and above since
Debian package version 2.5.1-1, but its CPP
conditionals were not prepared for that "mark
ref_module static" change being backported to stable
kernel lines.
Patch 2 (cherry-picked 352cdb28) is not included in
Debian Unstable and Bullseye as it is only necessary
for kernels older than those in Unstable/Bullseye which
got that change from 5.9 backported.
[ Changes ]
The proposed packages fixes #990123 by cherry-picking two upstream
commits in the same part of the code (I didn't want to merge them for
transparency reasons):
1) adfc6318 from Obtober 2020 which initially fixes this issue in
kernel 5.9.
Origin: adfc631816ea690cbf53c03a9f40b6c4c5be0a21
Author: ABC <abc@openwall.com>
Description: Fix compilation for 5.9: workaround ref_module unexport
compat.h:173:21: error: implicit declaration of function `ref_module' [-Werror=implicit-function-declaration]
# define use_module ref_module
^
ipt_NETFLOW.c:5488:3: note: in expansion of macro `use_module'
use_module(THIS_MODULE, netlink_m);
^~~~~~~~~~
Bug: https://github.com/aabc/ipt-netflow/issues/153
2) 352cdb28 from June 2021 which removes the restrictions that only
applied the fixes from adfc6318 to kernel 5.9 and above.
Origin: 352cdb28eecbb57de3509b18dfc37dcce0455c01
Author: ABC <abc@openwall.com>
Description: Fix compile for stable kernels by not using 'ref_module'
`ref_module' unexport in 7ef5264de7732 ("modules: mark ref_module
static") is back-ported into stable kernels making old `#if
LINUX_VERSION_CODE' checks irrelevant or too complicated to update.
Do not use `ref_module' API at all since `try_module_get' is ancient
enough to use always.
Bug: https://github.com/aabc/ipt-netflow/issues/177
Bug-Debian: https://bugs.debian.org/990123
Together (and only together) they also fix this issue for stable
kernels where recently ref_module was made static as in 5.9 about a
year ago.
[ Other info ]
Minor DEP3 patch metadata style fixes plus adding the relevant Debian
bug report reference to the cherry-picked 352cdb28 patch happened
after the initial week-long testing of the package. See
https://salsa.debian.org/debian/iptables-netflow/-/commit/1dcc6e12 for
these minor patch-metadata-only changes.
The package with updated DEP3 patch metadata has been tested as well
on the same installations and same ways as mentioned above, but not
for such a long period.
diff -Nru iptables-netflow-2.3/debian/changelog iptables-netflow-2.3/debian/changelog
--- iptables-netflow-2.3/debian/changelog 2018-07-27 19:47:20.000000000 +0200
+++ iptables-netflow-2.3/debian/changelog 2021-06-22 18:00:10.000000000 +0200
@@ -1,3 +1,12 @@
+iptables-netflow (2.3-5+deb10u1) buster; urgency=high
+
+ * Fix DKMS build failure regression caused by Linux upstream changes in
+ the 4.19.191 kernel by cherry-picking ipt_NETFLOW upstream commits
+ adfc6318 (initial fix for kernel 5.9) and 352cdb28 (removing the
+ special casing for older kernels). (Closes: #990123)
+
+ -- Axel Beckert <abe@debian.org> Tue, 22 Jun 2021 18:00:10 +0200
+
iptables-netflow (2.3-5) unstable; urgency=medium
* Add missing dependency (not build-dependency) on libc6-dev for DKMS
diff -Nru iptables-netflow-2.3/debian/patches/cherry-pick-352cdb28-Fix-compile-for-stable-kernels-by-not-using-ref_module.patch iptables-netflow-2.3/debian/patches/cherry-pick-352cdb28-Fix-compile-for-stable-kernels-by-not-using-ref_module.patch
--- iptables-netflow-2.3/debian/patches/cherry-pick-352cdb28-Fix-compile-for-stable-kernels-by-not-using-ref_module.patch 1970-01-01 01:00:00.000000000 +0100
+++ iptables-netflow-2.3/debian/patches/cherry-pick-352cdb28-Fix-compile-for-stable-kernels-by-not-using-ref_module.patch 2021-06-22 18:00:10.000000000 +0200
@@ -0,0 +1,58 @@
+Origin: 352cdb28eecbb57de3509b18dfc37dcce0455c01
+Author: ABC <abc@openwall.com>
+Description: Fix compile for stable kernels by not using 'ref_module'
+
+`ref_module' unexport in 7ef5264de7732 ("modules: mark ref_module
+static") is back-ported into stable kernels making old `#if
+LINUX_VERSION_CODE' checks irrelevant or too complicated to update.
+
+Do not use `ref_module' API at all since `try_module_get' is ancient
+enough to use always.
+
+Bug: https://github.com/aabc/ipt-netflow/issues/177
+Bug-Debian: https://bugs.debian.org/990123
+---
+ compat.h | 4 ----
+ ipt_NETFLOW.c | 7 +------
+ 2 files changed, 1 insertion(+), 10 deletions(-)
+
+--- a/compat.h
++++ b/compat.h
+@@ -169,10 +169,6 @@
+ # define CHECK_OK 0
+ #endif
+
+-#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,35)
+-# define use_module ref_module
+-#endif
+-
+ #ifndef NF_IP_LOCAL_IN /* 2.6.25 */
+ # define NF_IP_PRE_ROUTING NF_INET_PRE_ROUTING
+ # define NF_IP_LOCAL_IN NF_INET_LOCAL_IN
+--- a/ipt_NETFLOW.c
++++ b/ipt_NETFLOW.c
+@@ -5395,12 +5395,8 @@
+ }
+ /* Reference netlink module to prevent it's unsafe unload before us. */
+ if (!netlink_m && (netlink_m = find_module(NETLINK_M))) {
+-#if LINUX_VERSION_CODE < KERNEL_VERSION(5,9,0)
+- use_module(THIS_MODULE, netlink_m);
+-#else
+ if (!try_module_get(netlink_m))
+ netlink_m = NULL;
+-#endif
+ }
+
+ /* Register ct events callback. */
+@@ -5428,10 +5424,9 @@
+ #else /* < v3.2 */
+ unset_notifier_cb();
+ #endif /* v3.2 */
+-#if LINUX_VERSION_CODE >= KERNEL_VERSION(5,9,0)
+ module_put(netlink_m);
+ netlink_m = NULL;
+-#endif
++
+ rcu_assign_pointer(saved_event_cb, NULL);
+ #else /* < v2.6.31 */
+ nf_conntrack_unregister_notifier(&ctnl_notifier);
diff -Nru iptables-netflow-2.3/debian/patches/cherry-pick-adfc6318-fix-compilation-for-5.9-workaround-ref_module-unexport.patch iptables-netflow-2.3/debian/patches/cherry-pick-adfc6318-fix-compilation-for-5.9-workaround-ref_module-unexport.patch
--- iptables-netflow-2.3/debian/patches/cherry-pick-adfc6318-fix-compilation-for-5.9-workaround-ref_module-unexport.patch 1970-01-01 01:00:00.000000000 +0100
+++ iptables-netflow-2.3/debian/patches/cherry-pick-adfc6318-fix-compilation-for-5.9-workaround-ref_module-unexport.patch 2021-06-22 18:00:10.000000000 +0200
@@ -0,0 +1,66 @@
+Origin: adfc631816ea690cbf53c03a9f40b6c4c5be0a21
+Author: ABC <abc@openwall.com>
+Description: Fix compilation for 5.9: workaround ref_module unexport
+ compat.h:173:21: error: implicit declaration of function ‘ref_module’ [-Werror=implicit-function-declaration]
+ # define use_module ref_module
+ ^
+ ipt_NETFLOW.c:5488:3: note: in expansion of macro ‘use_module’
+ use_module(THIS_MODULE, netlink_m);
+ ^~~~~~~~~~
+Bug: https://github.com/aabc/ipt-netflow/issues/153
+
+---
+ ipt_NETFLOW.c | 18 +++++++++++++-----
+ 1 file changed, 13 insertions(+), 5 deletions(-)
+
+--- a/ipt_NETFLOW.c
++++ b/ipt_NETFLOW.c
+@@ -5370,13 +5370,12 @@
+ #endif /* since 2.6.31 */
+
+ static DEFINE_MUTEX(events_lock);
++static struct module *netlink_m;
+ /* Both functions may be called multiple times. */
+ static void register_ct_events(void)
+ {
+ #if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,31)
+ #define NETLINK_M "nf_conntrack_netlink"
+- struct module *netlink_m;
+- static int referenced = 0;
+ #endif
+
+ printk(KERN_INFO "ipt_NETFLOW: enable natevents.\n");
+@@ -5389,14 +5388,19 @@
+ #if LINUX_VERSION_CODE < KERNEL_VERSION(3,2,0)
+ !rcu_dereference(nf_conntrack_event_cb) ||
+ #endif
+- !(netlink_m = find_module(NETLINK_M))) {
++ !find_module(NETLINK_M)) {
+ printk("Loading " NETLINK_M "\n");
+ request_module(NETLINK_M);
++
+ }
+ /* Reference netlink module to prevent it's unsafe unload before us. */
+- if (!referenced && (netlink_m = find_module(NETLINK_M))) {
+- referenced++;
++ if (!netlink_m && (netlink_m = find_module(NETLINK_M))) {
++#if LINUX_VERSION_CODE < KERNEL_VERSION(5,9,0)
+ use_module(THIS_MODULE, netlink_m);
++#else
++ if (!try_module_get(netlink_m))
++ netlink_m = NULL;
++#endif
+ }
+
+ /* Register ct events callback. */
+@@ -5424,6 +5428,10 @@
+ #else /* < v3.2 */
+ unset_notifier_cb();
+ #endif /* v3.2 */
++#if LINUX_VERSION_CODE >= KERNEL_VERSION(5,9,0)
++ module_put(netlink_m);
++ netlink_m = NULL;
++#endif
+ rcu_assign_pointer(saved_event_cb, NULL);
+ #else /* < v2.6.31 */
+ nf_conntrack_unregister_notifier(&ctnl_notifier);
diff -Nru iptables-netflow-2.3/debian/patches/series iptables-netflow-2.3/debian/patches/series
--- iptables-netflow-2.3/debian/patches/series 2018-03-24 01:01:06.000000000 +0100
+++ iptables-netflow-2.3/debian/patches/series 2021-06-22 18:00:10.000000000 +0200
@@ -2,3 +2,5 @@
add-quoting-needed-by-dh_dkms.patch
properly-pass-CPPFLAGS-and-LDFLAGS.patch
disable-kernel-check.patch
+cherry-pick-adfc6318-fix-compilation-for-5.9-workaround-ref_module-unexport.patch
+cherry-pick-352cdb28-Fix-compile-for-stable-kernels-by-not-using-ref_module.patch
Reply to: