diff mbox series

[ebtables,2/2] configure.ac: add option --enable-kernel-64-userland-32

Message ID 20210518181730.13436-2-patrickdepinguin@gmail.com
State Changes Requested
Delegated to: Pablo Neira
Headers show
Series [ebtables,1/2] ebtables.h: restore KERNEL_64_USERSPACE_32 checks | expand

Commit Message

Thomas De Schampheleire May 18, 2021, 6:17 p.m. UTC 
From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

The ebtables build system seems to assume that 'sparc64' is the
only case where KERNEL_64_USERSPACE_32 is relevant, but this is not true.
This situation can happen on many architectures, especially in embedded
systems. For example, an Aarch64 processor with kernel in 64-bit but
userland built for 32-bit Arm. Or a 64-bit MIPS Octeon III processor, with
userland running in the 'n32' ABI.

While it is possible to set CFLAGS in the environment when calling the
configure script, the caller would need to know to not only specify
KERNEL_64_USERSPACE_32 but also the EBT_MIN_ALIGN value.

Instead, add a configure option. All internal details can then be handled by
the configure script.

Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
---
 configure.ac | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

Comments

Pablo Neira Ayuso May 24, 2021, 3:26 p.m. UTC | #1 
On Tue, May 18, 2021 at 08:17:30PM +0200, Thomas De Schampheleire wrote:
> From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> 
> The ebtables build system seems to assume that 'sparc64' is the
> only case where KERNEL_64_USERSPACE_32 is relevant, but this is not true.
> This situation can happen on many architectures, especially in embedded
> systems. For example, an Aarch64 processor with kernel in 64-bit but
> userland built for 32-bit Arm. Or a 64-bit MIPS Octeon III processor, with
> userland running in the 'n32' ABI.
> 
> While it is possible to set CFLAGS in the environment when calling the
> configure script, the caller would need to know to not only specify
> KERNEL_64_USERSPACE_32 but also the EBT_MIN_ALIGN value.
> 
> Instead, add a configure option. All internal details can then be handled by
> the configure script.

Are you enabling

CONFIG_NETFILTER_XTABLES_COMPAT

in your kernel build?

KERNEL_64_USERSPACE_32 was deprecated long time ago in favour of
CONFIG_NETFILTER_XTABLES_COMPAT.
Thomas De Schampheleire May 25, 2021, 11:52 a.m. UTC | #2 
Hello,

El lun, 24 may 2021 a las 17:26, Pablo Neira Ayuso
(<pablo@netfilter.org>) escribió:
>
> On Tue, May 18, 2021 at 08:17:30PM +0200, Thomas De Schampheleire wrote:
> > From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> >
> > The ebtables build system seems to assume that 'sparc64' is the
> > only case where KERNEL_64_USERSPACE_32 is relevant, but this is not true.
> > This situation can happen on many architectures, especially in embedded
> > systems. For example, an Aarch64 processor with kernel in 64-bit but
> > userland built for 32-bit Arm. Or a 64-bit MIPS Octeon III processor, with
> > userland running in the 'n32' ABI.
> >
> > While it is possible to set CFLAGS in the environment when calling the
> > configure script, the caller would need to know to not only specify
> > KERNEL_64_USERSPACE_32 but also the EBT_MIN_ALIGN value.
> >
> > Instead, add a configure option. All internal details can then be handled by
> > the configure script.
>
> Are you enabling
>
> CONFIG_NETFILTER_XTABLES_COMPAT
>
> in your kernel build?
>
> KERNEL_64_USERSPACE_32 was deprecated long time ago in favour of
> CONFIG_NETFILTER_XTABLES_COMPAT.

The option you refer to (CONFIG_NETFILTER_XTABLES_COMPAT) was
introduced with commit 47a6959fa331fe892a4fc3b48ca08e92045c6bda
(5.13-rc1). Before that point, it seems CONFIG_COMPAT was the relevant
flag. The checks on CONFIG_COMPAT were already introduced with commit
81e675c227ec60a0bdcbb547dc530ebee23ff931 in 2.6.34.x.

I have seen this problem on Linux 4.1 and 4.9, on an Aarch64 CPU with
64-bit kernel and userspace compiled as 32-bit ARM. In both kernels,
CONFIG_COMPAT was set.

So I am a bit surprised that I bump into this issue after upgrading
ebtables from 2.0.10-4 to 2.0.11 where the padding was removed.
According to your mail and the commits mentioned, it is supposed to
work without ebtables making specific provisions for the 32/64 bit
type difference.

When I apply the patches I submitted to this list, I get correct
behavior. Without them, the kernel complains and ebtables fails.

Best regards,
Thomas
Pablo Neira Ayuso May 27, 2021, 7:30 p.m. UTC | #3 
On Tue, May 25, 2021 at 01:52:27PM +0200, Thomas De Schampheleire wrote:
> Hello,
> 
> El lun, 24 may 2021 a las 17:26, Pablo Neira Ayuso
> (<pablo@netfilter.org>) escribió:
> >
> > On Tue, May 18, 2021 at 08:17:30PM +0200, Thomas De Schampheleire wrote:
> > > From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> > >
> > > The ebtables build system seems to assume that 'sparc64' is the
> > > only case where KERNEL_64_USERSPACE_32 is relevant, but this is not true.
> > > This situation can happen on many architectures, especially in embedded
> > > systems. For example, an Aarch64 processor with kernel in 64-bit but
> > > userland built for 32-bit Arm. Or a 64-bit MIPS Octeon III processor, with
> > > userland running in the 'n32' ABI.
> > >
> > > While it is possible to set CFLAGS in the environment when calling the
> > > configure script, the caller would need to know to not only specify
> > > KERNEL_64_USERSPACE_32 but also the EBT_MIN_ALIGN value.
> > >
> > > Instead, add a configure option. All internal details can then be handled by
> > > the configure script.
> >
> > Are you enabling
> >
> > CONFIG_NETFILTER_XTABLES_COMPAT
> >
> > in your kernel build?
> >
> > KERNEL_64_USERSPACE_32 was deprecated long time ago in favour of
> > CONFIG_NETFILTER_XTABLES_COMPAT.
> 
> The option you refer to (CONFIG_NETFILTER_XTABLES_COMPAT) was
> introduced with commit 47a6959fa331fe892a4fc3b48ca08e92045c6bda
> (5.13-rc1). Before that point, it seems CONFIG_COMPAT was the relevant
> flag.

Sorry, I got confused by this recent commit, it's indeed CONFIG_COMPAT
the right toggle in old kernels.

> The checks on CONFIG_COMPAT were already introduced with commit
> 81e675c227ec60a0bdcbb547dc530ebee23ff931 in 2.6.34.x.
> 
> I have seen this problem on Linux 4.1 and 4.9, on an Aarch64 CPU with
> 64-bit kernel and userspace compiled as 32-bit ARM. In both kernels,
> CONFIG_COMPAT was set.

Hm, then ebtables compat is buggy.

> So I am a bit surprised that I bump into this issue after upgrading
> ebtables from 2.0.10-4 to 2.0.11 where the padding was removed.
> According to your mail and the commits mentioned, it is supposed to
> work without ebtables making specific provisions for the 32/64 bit
> type difference.
> 
> When I apply the patches I submitted to this list, I get correct
> behavior. Without them, the kernel complains and ebtables fails.

I understand. If this old userspace infrastructure is restored, then
ebtables compat kernel might not ever be fixed.
Florian Westphal May 28, 2021, 5:10 p.m. UTC | #4 
Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > introduced with commit 47a6959fa331fe892a4fc3b48ca08e92045c6bda
> > (5.13-rc1). Before that point, it seems CONFIG_COMPAT was the relevant
> > flag.
> 
> Sorry, I got confused by this recent commit, it's indeed CONFIG_COMPAT
> the right toggle in old kernels.
> 
> > The checks on CONFIG_COMPAT were already introduced with commit
> > 81e675c227ec60a0bdcbb547dc530ebee23ff931 in 2.6.34.x.
> > 
> > I have seen this problem on Linux 4.1 and 4.9, on an Aarch64 CPU with
> > 64-bit kernel and userspace compiled as 32-bit ARM. In both kernels,
> > CONFIG_COMPAT was set.
> 
> Hm, then ebtables compat is buggy.

It was only ever tested with i686 binary on amd64 arch.

Thomas, does unmodified 32bit iptables work on those arch/kernel
combinations?

> > So I am a bit surprised that I bump into this issue after upgrading
> > ebtables from 2.0.10-4 to 2.0.11 where the padding was removed.
> > According to your mail and the commits mentioned, it is supposed to
> > work without ebtables making specific provisions for the 32/64 bit
> > type difference.

ebtables-userspace compat fixups predate the ebtables kernel side
support, it was autoenabled on sparc64 in the old makefile:

ifeq ($(shell uname -m),sparc64)
CFLAGS+=-DEBT_MIN_ALIGN=8 -DKERNEL_64_USERSPACE_32
endif

I don't even know if the ebtables compat support is compiled in on
non-amd64.
Thomas De Schampheleire May 31, 2021, 12:11 p.m. UTC | #5 
Hello,

El vie, 28 may 2021 a las 19:10, Florian Westphal (<fw@strlen.de>) escribió:
>
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > introduced with commit 47a6959fa331fe892a4fc3b48ca08e92045c6bda
> > > (5.13-rc1). Before that point, it seems CONFIG_COMPAT was the relevant
> > > flag.
> >
> > Sorry, I got confused by this recent commit, it's indeed CONFIG_COMPAT
> > the right toggle in old kernels.
> >
> > > The checks on CONFIG_COMPAT were already introduced with commit
> > > 81e675c227ec60a0bdcbb547dc530ebee23ff931 in 2.6.34.x.
> > >
> > > I have seen this problem on Linux 4.1 and 4.9, on an Aarch64 CPU with
> > > 64-bit kernel and userspace compiled as 32-bit ARM. In both kernels,
> > > CONFIG_COMPAT was set.
> >
> > Hm, then ebtables compat is buggy.
>
> It was only ever tested with i686 binary on amd64 arch.

I have verified now again with the same procedure, i.e. build ebtables
2.0.11 without proposed patches or special flags, on following
platforms:

1.  x86_64 kernel 5.4.x + i686 userspace: ebtables works correctly

2.  aarch64 kernel 4.1.x + 32-bit ARM userspace: ebtables fails as described

As mentioned before, in both cases CONFIG_COMPAT=y .


>
> Thomas, does unmodified 32bit iptables work on those arch/kernel
> combinations?

Yes, iptables 1.8.6 is used successfully without special provisioning
for bitness. We are using Buildroot 2021.02 to compile.

>
> > > So I am a bit surprised that I bump into this issue after upgrading
> > > ebtables from 2.0.10-4 to 2.0.11 where the padding was removed.
> > > According to your mail and the commits mentioned, it is supposed to
> > > work without ebtables making specific provisions for the 32/64 bit
> > > type difference.
>
> ebtables-userspace compat fixups predate the ebtables kernel side
> support, it was autoenabled on sparc64 in the old makefile:
>
> ifeq ($(shell uname -m),sparc64)
> CFLAGS+=-DEBT_MIN_ALIGN=8 -DKERNEL_64_USERSPACE_32
> endif

Yes, in the proposed changes to ebtables userspace, this kind of logic
is restored, but not based on the machine type but with an autoconf
flag.

>
> I don't even know if the ebtables compat support is compiled in on
> non-amd64.


Can you be more specific what you are referring to here?

For the kernel part, a long time ago you already created commit
81e675c227ec60a0bdcbb547dc530ebee23ff931 which is supposed to add
compatibility when CONFIG_COMPAT=y. This code is still present in the
4.1.x I tested above.

So at this moment it seems to me that the kernel compat support is
effectively compiled in, and supports x86(_64) but does not support
the Aarch64/ARM combination (and perhaps others).

How to proceed now?

Thanks,
Thomas
Florian Westphal June 1, 2021, 2:50 p.m. UTC | #6 
Thomas De Schampheleire <patrickdepinguin@gmail.com> wrote:
> 1.  x86_64 kernel 5.4.x + i686 userspace: ebtables works correctly
> 
> 2.  aarch64 kernel 4.1.x + 32-bit ARM userspace: ebtables fails as described
> 
> As mentioned before, in both cases CONFIG_COMPAT=y .
> >
> > Thomas, does unmodified 32bit iptables work on those arch/kernel
> > combinations?
> 
> Yes, iptables 1.8.6 is used successfully without special provisioning
> for bitness. We are using Buildroot 2021.02 to compile.

Ok, so this is 'just' a bug in the ebtables translation layer.

Its likely that there are alignment differences on aarch that the
ebtables i686 fixups are not aware of.

> > ebtables-userspace compat fixups predate the ebtables kernel side
> > support, it was autoenabled on sparc64 in the old makefile:
> >
> > ifeq ($(shell uname -m),sparc64)
> > CFLAGS+=-DEBT_MIN_ALIGN=8 -DKERNEL_64_USERSPACE_32
> > endif
> 
> Yes, in the proposed changes to ebtables userspace, this kind of logic
> is restored, but not based on the machine type but with an autoconf
> flag.
> 
> >
> > I don't even know if the ebtables compat support is compiled in on
> > non-amd64.
> 
> Can you be more specific what you are referring to here?

I meant I wasn't sure if the ebtables compat stuff is compiled in on
non-amd64 platforms.  But I guess they are because iptables works for
you.

> So at this moment it seems to me that the kernel compat support is
> effectively compiled in, and supports x86(_64) but does not support
> the Aarch64/ARM combination (and perhaps others).
> 
> How to proceed now?

The proper solution is to make the existing translation work on aarch64.

It will take me some time to get a crosscompiler+qemu setup going
though.
diff mbox series

Patch

diff --git a/configure.ac b/configure.ac
index c24ede3..3e89c0c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -15,10 +15,17 @@  AS_IF([test "x$LOCKFILE" = x], [LOCKFILE="/var/lib/ebtables/lock"])
 
 regular_CFLAGS="-Wall -Wunused"
 regular_CPPFLAGS=""
+
 case "$host" in
 	sparc64-*)
-		regular_CPPFLAGS="$regular_CPPFLAGS -DEBT_MIN_ALIGN=8 -DKERNEL_64_USERSPACE_32";;
+		enable_kernel_64_userland_32=yes ;;
 esac
+AC_ARG_ENABLE([kernel-64-userland-32],
+    AC_HELP_STRING([--enable-kernel-64-userland-32], [indicate that ebtables will be built as a 32-bit application but run under a 64-bit kernel])
+)
+AS_IF([test "x$enable_kernel_64_userland_32" = xyes],
+    [regular_CPPFLAGS="$regular_CPPFLAGS -DEBT_MIN_ALIGN=8 -DKERNEL_64_USERSPACE_32"]
+)
 
 AC_SUBST([regular_CFLAGS])
 AC_SUBST([regular_CPPFLAGS])