diff mbox series

[1/1] package/ebtables: fix build with gcc >= 14

Message ID 20240724071904.1397662-1-fontaine.fabrice@gmail.com
State New
Headers show
Series [1/1] package/ebtables: fix build with gcc >= 14 | expand

Commit Message

Fabrice Fontaine July 24, 2024, 7:19 a.m. UTC
Drop second patch which has not been accepted by upstream and raises the
following build failure with gcc >= 14:

communication.c: In function 'ebt_deliver_table':
communication.c:252:26: error: passing argument 1 of 'free' makes pointer from integer without a cast [-Wint-conversion]
  252 |                 free(repl->entries);
      |                      ~~~~^~~~~~~~~
      |                          |
      |                          uint64_t {aka long unsigned int}
In file included from communication.c:19:
/home/autobuild/autobuild/instance-11/output-1/host/sparc64-buildroot-linux-gnu/sysroot/usr/include/stdlib.h:687:25: note: expected 'void *' but argument is of type 'uint64_t' {aka 'long unsigned int'}
  687 | extern void free (void *__ptr) __THROW;
      |                   ~~~~~~^~~~~
communication.c: In function 'retrieve_from_file':
communication.c:653:31: error: assignment to 'uint64_t' {aka 'long unsigned int'} from 'void *' makes integer from pointer without a cast [-Wint-conversion]
  653 |                 repl->entries = NULL;
      |                               ^
communication.c: In function 'ebt_get_table':
communication.c:730:26: error: assignment to 'struct ebt_counter *' from 'uint64_t' {aka 'long unsigned int'} makes pointer from integer without a cast [-Wint-conversion]
  730 |         u_repl->counters = repl.counters;
      |                          ^
communication.c:770:18: error: passing argument 1 of 'free' makes pointer from integer without a cast [-Wint-conversion]
  770 |         free(repl.entries);
      |              ~~~~^~~~~~~~
      |                  |
      |                  uint64_t {aka long unsigned int}
/home/autobuild/autobuild/instance-11/output-1/host/sparc64-buildroot-linux-gnu/sysroot/usr/include/stdlib.h:687:25: note: expected 'void *' but argument is of type 'uint64_t' {aka 'long unsigned int'}
  687 | extern void free (void *__ptr) __THROW;
      |                   ~~~~~~^~~~~

Those build failures are raised because ebt_replace is patched to change
type of entries and counters depending on the value of
KERNEL_64_USERSPACE_32. However, this patch is broken as it doesn't
update all the source code to avoid calling free on uint64_t or setting
NULL to uint64_t. More dramatically, ebt_u_replace structure has not
been patched either so uint64_t is copied into struct ebt_counter *.

Fixing all this mess seems complicated especially given the fact that
upstream rejected second and third patches.

So to fix this build failure:
 - always set --disable-kernel-64-userland-32 (even on sparc64 where
   the build will fail due to the incorrect "sparc_cast" remaining in
   the source code)
 - add a dependency on !BR2_KERNEL_64_USERLAND_32. This will disable
   ebtables only with BR2_MIPS_NABI32 as this is the only architecture
   that sets BR2_KERNEL_64_USERLAND_32 and so the only architecture
   (with sparc64) that was using this "broken" patch

While at it, fix Upstream tag on patches

Fixes: 4b5743e5235ff7d88806305a40476ecb9b408876
 - http://autobuild.buildroot.org/results/ae9b07a88a43b381bd8ce5dca9f434f63e1dcd49
 - http://autobuild.buildroot.org/results/be20febfb9e21bee5c22138e6a34016079b7eab8

Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
---
 .checkpackageignore                           |   3 -
 ...-ebtables-save-perl-script-with-bash.patch |   3 +-
 ...option-enable-kernel-64-userland-32.patch} |   7 +-
 ...estore-KERNEL_64_USERSPACE_32-checks.patch | 105 ------------------
 package/ebtables/Config.in                    |   1 +
 package/ebtables/ebtables.mk                  |   7 +-
 6 files changed, 9 insertions(+), 117 deletions(-)
 rename package/ebtables/{0003-configure.ac-add-option-enable-kernel-64-userland-32.patch => 0002-configure.ac-add-option-enable-kernel-64-userland-32.patch} (85%)
 delete mode 100644 package/ebtables/0002-ebtables.h-restore-KERNEL_64_USERSPACE_32-checks.patch

Comments

Thomas Petazzoni July 24, 2024, 4:28 p.m. UTC | #1
Hello Fabrice,

Thanks for looking into this, it looks complicated. Question below.

On Wed, 24 Jul 2024 09:19:04 +0200
Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:

> diff --git a/package/ebtables/0003-configure.ac-add-option-enable-kernel-64-userland-32.patch b/package/ebtables/0002-configure.ac-add-option-enable-kernel-64-userland-32.patch
> similarity index 85%
> rename from package/ebtables/0003-configure.ac-add-option-enable-kernel-64-userland-32.patch
> rename to package/ebtables/0002-configure.ac-add-option-enable-kernel-64-userland-32.patch
> index cb57b39569..7e481606e7 100644
> --- a/package/ebtables/0003-configure.ac-add-option-enable-kernel-64-userland-32.patch
> +++ b/package/ebtables/0002-configure.ac-add-option-enable-kernel-64-userland-32.patch

So why do we have that adds an option to enable kernel-64-userland-32, if...

> -# for 0003-configure.ac-add-option-enable-kernel-64-userland-32.patch
> +# for 0002-configure.ac-add-option-enable-kernel-64-userland-32.patch
>  EBTABLES_AUTORECONF = YES
> -ifeq ($(BR2_KERNEL_64_USERLAND_32),y)
> -EBTABLES_CONF_OPTS += --enable-kernel-64-userland-32
> -endif
> +# kernel-64-userland-32 is broken (spotted with gcc >= 14)
> +EBTABLES_CONF_OPTS += --disable-kernel-64-userland-32

... we always disable it? Doesn't this make the patch entirely useless?

Thanks,

Thomas
Thomas Petazzoni July 24, 2024, 4:29 p.m. UTC | #2
Hello,

Another short comment.

On Wed, 24 Jul 2024 09:19:04 +0200
Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:

> diff --git a/package/ebtables/Config.in b/package/ebtables/Config.in
> index 703e55a3c3..ea06b0d8b5 100644
> --- a/package/ebtables/Config.in
> +++ b/package/ebtables/Config.in
> @@ -1,6 +1,7 @@
>  config BR2_PACKAGE_EBTABLES
>  	bool "ebtables"
>  	depends on BR2_USE_MMU # fork()
> +	depends on !BR2_KERNEL_64_USERLAND_32

As this is a super unusual dependency, we clearly want to have a
comment above it (not a Config.in comment, a source comment) that
explains why we have this strange dependency.

Thanks!

Thomas
Thomas Petazzoni Aug. 2, 2024, 5:20 p.m. UTC | #3
Hello Fabrice,

Any feedback to my review on this patch (this e-mail, and the other
reply to your patch) ? This would allow to move forward with merging
this fix. Thanks a lot!

Thomas

On Wed, 24 Jul 2024 18:28:38 +0200
Thomas Petazzoni via buildroot <buildroot@buildroot.org> wrote:

> Hello Fabrice,
> 
> Thanks for looking into this, it looks complicated. Question below.
> 
> On Wed, 24 Jul 2024 09:19:04 +0200
> Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:
> 
> > diff --git a/package/ebtables/0003-configure.ac-add-option-enable-kernel-64-userland-32.patch b/package/ebtables/0002-configure.ac-add-option-enable-kernel-64-userland-32.patch
> > similarity index 85%
> > rename from package/ebtables/0003-configure.ac-add-option-enable-kernel-64-userland-32.patch
> > rename to package/ebtables/0002-configure.ac-add-option-enable-kernel-64-userland-32.patch
> > index cb57b39569..7e481606e7 100644
> > --- a/package/ebtables/0003-configure.ac-add-option-enable-kernel-64-userland-32.patch
> > +++ b/package/ebtables/0002-configure.ac-add-option-enable-kernel-64-userland-32.patch  
> 
> So why do we have that adds an option to enable kernel-64-userland-32, if...
> 
> > -# for 0003-configure.ac-add-option-enable-kernel-64-userland-32.patch
> > +# for 0002-configure.ac-add-option-enable-kernel-64-userland-32.patch
> >  EBTABLES_AUTORECONF = YES
> > -ifeq ($(BR2_KERNEL_64_USERLAND_32),y)
> > -EBTABLES_CONF_OPTS += --enable-kernel-64-userland-32
> > -endif
> > +# kernel-64-userland-32 is broken (spotted with gcc >= 14)
> > +EBTABLES_CONF_OPTS += --disable-kernel-64-userland-32  
> 
> ... we always disable it? Doesn't this make the patch entirely useless?
> 
> Thanks,
> 
> Thomas
diff mbox series

Patch

diff --git a/.checkpackageignore b/.checkpackageignore
index 5a2d96a274..1372ae1f2b 100644
--- a/.checkpackageignore
+++ b/.checkpackageignore
@@ -476,9 +476,6 @@  package/dvdrw-tools/0001-limits.h.patch lib_patch.Upstream
 package/dvdrw-tools/0002-Include-sysmacros.h-to-compile-with-newer-gcc.patch lib_patch.Upstream
 package/earlyoom/0001-main.c-fix-build-with-kernel-4.3.patch lib_patch.Upstream
 package/earlyoom/S02earlyoom Shellcheck lib_sysv.Indent
-package/ebtables/0001-replace-ebtables-save-perl-script-with-bash.patch lib_patch.Upstream
-package/ebtables/0002-ebtables.h-restore-KERNEL_64_USERSPACE_32-checks.patch lib_patch.Upstream
-package/ebtables/0003-configure.ac-add-option-enable-kernel-64-userland-32.patch lib_patch.Upstream
 package/ecryptfs-utils/0001-musl.patch lib_patch.Upstream
 package/ecryptfs-utils/0002-openssl110.patch lib_patch.Upstream
 package/ecryptfs-utils/0003-fix-parallel-build-issue.patch lib_patch.Upstream
diff --git a/package/ebtables/0001-replace-ebtables-save-perl-script-with-bash.patch b/package/ebtables/0001-replace-ebtables-save-perl-script-with-bash.patch
index 525e8a28b5..3cb3c3b6e4 100644
--- a/package/ebtables/0001-replace-ebtables-save-perl-script-with-bash.patch
+++ b/package/ebtables/0001-replace-ebtables-save-perl-script-with-bash.patch
@@ -12,8 +12,7 @@  already.
   https://bugzilla.redhat.com/show_bug.cgi?id=746040
   http://pkgs.fedoraproject.org/cgit/rpms/ebtables.git/tree/ebtables-save
 
-Upstream:
-https://github.com/openembedded/meta-openembedded/commit/7f723007364ba79de05447671e83d4eefb3097dc
+Upstream: https://github.com/openembedded/meta-openembedded/commit/7f723007364ba79de05447671e83d4eefb3097dc
 
 Signed-off-by: Matthew Weber <matthew.weber@rockwellcollins.com>
 [ryanbarnett3@gmail.com:
diff --git a/package/ebtables/0003-configure.ac-add-option-enable-kernel-64-userland-32.patch b/package/ebtables/0002-configure.ac-add-option-enable-kernel-64-userland-32.patch
similarity index 85%
rename from package/ebtables/0003-configure.ac-add-option-enable-kernel-64-userland-32.patch
rename to package/ebtables/0002-configure.ac-add-option-enable-kernel-64-userland-32.patch
index cb57b39569..7e481606e7 100644
--- a/package/ebtables/0003-configure.ac-add-option-enable-kernel-64-userland-32.patch
+++ b/package/ebtables/0002-configure.ac-add-option-enable-kernel-64-userland-32.patch
@@ -18,7 +18,9 @@  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>
-Upstream-Status: http://patchwork.ozlabs.org/project/netfilter-devel/patch/20210518181730.13436-2-patrickdepinguin@gmail.com/
+Upstream: http://patchwork.ozlabs.org/project/netfilter-devel/patch/20210518181730.13436-2-patrickdepinguin@gmail.com/
+[Fabrice: Don't force kernel-64-userland-32 with sparc64]
+Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
 ---
  configure.ac | 9 ++++++++-
  1 file changed, 8 insertions(+), 1 deletion(-)
@@ -27,7 +29,7 @@  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"])
+@@ -15,10 +15,16 @@ AS_IF([test "x$LOCKFILE" = x], [LOCKFILE="/var/lib/ebtables/lock"])
  
  regular_CFLAGS="-Wall -Wunused"
  regular_CPPFLAGS=""
@@ -35,7 +37,6 @@  index c24ede3..3e89c0c 100644
  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])
diff --git a/package/ebtables/0002-ebtables.h-restore-KERNEL_64_USERSPACE_32-checks.patch b/package/ebtables/0002-ebtables.h-restore-KERNEL_64_USERSPACE_32-checks.patch
deleted file mode 100644
index 84b4d0f392..0000000000
--- a/package/ebtables/0002-ebtables.h-restore-KERNEL_64_USERSPACE_32-checks.patch
+++ /dev/null
@@ -1,105 +0,0 @@ 
-From 7297a8ef3cab3b0faf1426622ee902a2144e2e89 Mon Sep 17 00:00:00 2001
-From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
-Date: Wed, 24 Mar 2021 11:27:14 +0100
-Subject: [PATCH] ebtables.h: restore KERNEL_64_USERSPACE_32 checks
-
-Commit e6359eedfbf497e52d52451072aea4713ed80a88 replaced the file ebtables.h
-but removed the usage of KERNEL_64_USERSPACE_32. This breaks boards where
-such flag is relevant, with following messages:
-
-[ 6364.971346] kernel msg: ebtables bug: please report to author: Standard target size too big
-
-Unable to update the kernel. Two possible causes:
-1. Multiple ebtables programs were executing simultaneously. The ebtables
-   userspace tool doesn't by default support multiple ebtables programs running
-   concurrently. The ebtables option --concurrent or a tool like flock can be
-   used to support concurrent scripts that update the ebtables kernel tables.
-2. The kernel doesn't support a certain ebtables extension, consider
-   recompiling your kernel or insmod the extension.
-
-Analysis shows that the structure 'ebt_replace' passed from userspace
-ebtables to the kernel, is too small, i.e 80 bytes instead of 120 in case of
-64-bit kernel.
-
-Note that 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 build for 32-bit Arm. Or a 64-bit MIPS Octeon III processor, with
-userland running in the 'n32' ABI.
-
-Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
-Upstream-Status: http://patchwork.ozlabs.org/project/netfilter-devel/patch/20210518181730.13436-1-patrickdepinguin@gmail.com/
----
- include/linux/netfilter_bridge/ebtables.h | 21 +++++++++++++++++++++
- 1 file changed, 21 insertions(+)
-
-diff --git a/include/linux/netfilter_bridge/ebtables.h b/include/linux/netfilter_bridge/ebtables.h
-index 5be75f2..3c2b61e 100644
---- a/include/linux/netfilter_bridge/ebtables.h
-+++ b/include/linux/netfilter_bridge/ebtables.h
-@@ -49,12 +49,21 @@ struct ebt_replace {
- 	/* total size of the entries */
- 	unsigned int entries_size;
- 	/* start of the chains */
-+#ifdef KERNEL_64_USERSPACE_32
-+	uint64_t hook_entry[NF_BR_NUMHOOKS];
-+#else
- 	struct ebt_entries *hook_entry[NF_BR_NUMHOOKS];
-+#endif
- 	/* nr of counters userspace expects back */
- 	unsigned int num_counters;
- 	/* where the kernel will put the old counters */
-+#ifdef KERNEL_64_USERSPACE_32
-+	uint64_t counters;
-+	uint64_t entries;
-+#else
- 	struct ebt_counter *counters;
- 	char *entries;
-+#endif
- };
- 
- struct ebt_replace_kernel {
-@@ -129,6 +138,9 @@ struct ebt_entry_match {
- 	} u;
- 	/* size of data */
- 	unsigned int match_size;
-+#ifdef KERNEL_64_USERSPACE_32
-+	unsigned int pad;
-+#endif
- 	unsigned char data[0] __attribute__ ((aligned (__alignof__(struct ebt_replace))));
- };
- 
-@@ -142,6 +154,9 @@ struct ebt_entry_watcher {
- 	} u;
- 	/* size of data */
- 	unsigned int watcher_size;
-+#ifdef KERNEL_64_USERSPACE_32
-+	unsigned int pad;
-+#endif
- 	unsigned char data[0] __attribute__ ((aligned (__alignof__(struct ebt_replace))));
- };
- 
-@@ -155,6 +170,9 @@ struct ebt_entry_target {
- 	} u;
- 	/* size of data */
- 	unsigned int target_size;
-+#ifdef KERNEL_64_USERSPACE_32
-+	unsigned int pad;
-+#endif
- 	unsigned char data[0] __attribute__ ((aligned (__alignof__(struct ebt_replace))));
- };
- 
-@@ -162,6 +180,9 @@ struct ebt_entry_target {
- struct ebt_standard_target {
- 	struct ebt_entry_target target;
- 	int verdict;
-+#ifdef KERNEL_64_USERSPACE_32
-+	unsigned int pad;
-+#endif
- };
- 
- /* one entry */
--- 
-2.26.2
-
diff --git a/package/ebtables/Config.in b/package/ebtables/Config.in
index 703e55a3c3..ea06b0d8b5 100644
--- a/package/ebtables/Config.in
+++ b/package/ebtables/Config.in
@@ -1,6 +1,7 @@ 
 config BR2_PACKAGE_EBTABLES
 	bool "ebtables"
 	depends on BR2_USE_MMU # fork()
+	depends on !BR2_KERNEL_64_USERLAND_32
 	help
 	  Ethernet bridge frame table administration
 
diff --git a/package/ebtables/ebtables.mk b/package/ebtables/ebtables.mk
index 46c22dde9b..74048f6339 100644
--- a/package/ebtables/ebtables.mk
+++ b/package/ebtables/ebtables.mk
@@ -11,11 +11,10 @@  EBTABLES_LICENSE_FILES = COPYING
 EBTABLES_CPE_ID_VENDOR = netfilter
 EBTABLES_SELINUX_MODULES = iptables
 
-# for 0003-configure.ac-add-option-enable-kernel-64-userland-32.patch
+# for 0002-configure.ac-add-option-enable-kernel-64-userland-32.patch
 EBTABLES_AUTORECONF = YES
-ifeq ($(BR2_KERNEL_64_USERLAND_32),y)
-EBTABLES_CONF_OPTS += --enable-kernel-64-userland-32
-endif
+# kernel-64-userland-32 is broken (spotted with gcc >= 14)
+EBTABLES_CONF_OPTS += --disable-kernel-64-userland-32
 
 ifeq ($(BR2_PACKAGE_EBTABLES_UTILS_SAVE),y)
 define EBTABLES_INSTALL_TARGET_UTILS_SAVE