[PATCH v2] netfilter: nfacct: Fix alignment mismatch in xt_nfacct_match_info

[PATCH v2] netfilter: nfacct: Fix alignment mismatch in xt_nfacct_match_info

[Juliana Rodrigueiro]

When running a 64-bit kernel with a 32-bit iptables binary, the size of
the xt_nfacct_match_info struct diverges.

    kernel: sizeof(struct xt_nfacct_match_info) : 40
    iptables: sizeof(struct xt_nfacct_match_info)) : 36

Trying to append nfacct related rules results in an unhelpful message.
Although it is suggested to look for more information in dmesg, nothing
can be found there.

    # iptables -A <chain> -m nfacct --nfacct-name <acct-object>
    iptables: Invalid argument. Run `dmesg' for more information.

This patch fixes the memory misalignment by enforcing 8-byte alignment
within the struct's first revision. This solution is often used in many
other uapi netfilter headers.

Signed-off-by: Juliana Rodrigueiro <juliana.rodrigueiro@intra2net.com>
---
Changes in v2:
    - Keep ABI by creating a v1 of the match struct.

 include/uapi/linux/netfilter/xt_nfacct.h |  5 ++++
 net/netfilter/xt_nfacct.c                | 36 ++++++++++++++++--------
 2 files changed, 30 insertions(+), 11 deletions(-)

diff --git a/include/uapi/linux/netfilter/xt_nfacct.h b/include/uapi/linux/netfilter/xt_nfacct.h
index 5c8a4d760ee3..b5123ab8d54a 100644
--- a/include/uapi/linux/netfilter/xt_nfacct.h
+++ b/include/uapi/linux/netfilter/xt_nfacct.h
@@ -11,4 +11,9 @@ struct xt_nfacct_match_info {
 	struct nf_acct	*nfacct;
 };
 
+struct xt_nfacct_match_info_v1 {
+	char		name[NFACCT_NAME_MAX];
+	struct nf_acct	*nfacct __attribute__((aligned(8)));
+};
+
 #endif /* _XT_NFACCT_MATCH_H */
diff --git a/net/netfilter/xt_nfacct.c b/net/netfilter/xt_nfacct.c
index 6b56f4170860..3241fee9f2a1 100644
--- a/net/netfilter/xt_nfacct.c
+++ b/net/netfilter/xt_nfacct.c
@@ -57,25 +57,39 @@ nfacct_mt_destroy(const struct xt_mtdtor_param *par)
 	nfnl_acct_put(info->nfacct);
 }
 
-static struct xt_match nfacct_mt_reg __read_mostly = {
-	.name       = "nfacct",
-	.family     = NFPROTO_UNSPEC,
-	.checkentry = nfacct_mt_checkentry,
-	.match      = nfacct_mt,
-	.destroy    = nfacct_mt_destroy,
-	.matchsize  = sizeof(struct xt_nfacct_match_info),
-	.usersize   = offsetof(struct xt_nfacct_match_info, nfacct),
-	.me         = THIS_MODULE,
+static struct xt_match nfacct_mt_reg[] __read_mostly = {
+	{
+		.name       = "nfacct",
+		.revision   = 0,
+		.family     = NFPROTO_UNSPEC,
+		.checkentry = nfacct_mt_checkentry,
+		.match      = nfacct_mt,
+		.destroy    = nfacct_mt_destroy,
+		.matchsize  = sizeof(struct xt_nfacct_match_info),
+		.usersize   = offsetof(struct xt_nfacct_match_info, nfacct),
+		.me         = THIS_MODULE,
+	},
+	{
+		.name       = "nfacct",
+		.revision   = 1,
+		.family     = NFPROTO_UNSPEC,
+		.checkentry = nfacct_mt_checkentry,
+		.match      = nfacct_mt,
+		.destroy    = nfacct_mt_destroy,
+		.matchsize  = sizeof(struct xt_nfacct_match_info_v1),
+		.usersize   = offsetof(struct xt_nfacct_match_info_v1, nfacct),
+		.me         = THIS_MODULE,
+	},
 };
 
 static int __init nfacct_mt_init(void)
 {
-	return xt_register_match(&nfacct_mt_reg);
+	return xt_register_matches(nfacct_mt_reg, ARRAY_SIZE(nfacct_mt_reg));
 }
 
 static void __exit nfacct_mt_exit(void)
 {
-	xt_unregister_match(&nfacct_mt_reg);
+	xt_unregister_matches(nfacct_mt_reg, ARRAY_SIZE(nfacct_mt_reg));
 }
 
 module_init(nfacct_mt_init);
-- 
2.20.1

Re: [PATCH v2] netfilter: nfacct: Fix alignment mismatch in xt_nfacct_match_info

[Juliana Rodrigueiro]

Hi Florian.

I hope this patch reflects your suggestion to add a 'v1' match revision
to nfacct. To be sincere, I'm not sure if should have also written
nfacct_mt_v1() and etc, since these would be pretty much duplicate code.


Please let me know if this patch needs more work.

Best regards,
Juliana.

On 8/16/19 5:02 PM, Juliana Rodrigueiro wrote:
> When running a 64-bit kernel with a 32-bit iptables binary, the size of
> the xt_nfacct_match_info struct diverges.
> 
>      kernel: sizeof(struct xt_nfacct_match_info) : 40
>      iptables: sizeof(struct xt_nfacct_match_info)) : 36
> 
> Trying to append nfacct related rules results in an unhelpful message.
> Although it is suggested to look for more information in dmesg, nothing
> can be found there.
> 
>      # iptables -A <chain> -m nfacct --nfacct-name <acct-object>
>      iptables: Invalid argument. Run `dmesg' for more information.
> 
> This patch fixes the memory misalignment by enforcing 8-byte alignment
> within the struct's first revision. This solution is often used in many
> other uapi netfilter headers.
> 
> Signed-off-by: Juliana Rodrigueiro <juliana.rodrigueiro@intra2net.com>
> ---
> Changes in v2:
>      - Keep ABI by creating a v1 of the match struct.
> 
>   include/uapi/linux/netfilter/xt_nfacct.h |  5 ++++
>   net/netfilter/xt_nfacct.c                | 36 ++++++++++++++++--------
>   2 files changed, 30 insertions(+), 11 deletions(-)
> 

Re: [PATCH v2] netfilter: nfacct: Fix alignment mismatch in xt_nfacct_match_info

[Pablo Neira Ayuso]

On Fri, Aug 16, 2019 at 05:09:56PM +0200, Juliana Rodrigueiro wrote:
> Hi Florian.
> 
> I hope this patch reflects your suggestion to add a 'v1' match revision
> to nfacct. To be sincere, I'm not sure if should have also written
> nfacct_mt_v1() and etc, since these would be pretty much duplicate code.
> 
> Please let me know if this patch needs more work.

Please, send userspace iptables patch to add v1 too. Thanks.

Re: [PATCH v2] netfilter: nfacct: Fix alignment mismatch in xt_nfacct_match_info

[Florian Westphal]

Juliana Rodrigueiro <juliana.rodrigueiro@intra2net.com> wrote:
> When running a 64-bit kernel with a 32-bit iptables binary, the size of
> the xt_nfacct_match_info struct diverges.
> 
>     kernel: sizeof(struct xt_nfacct_match_info) : 40
>     iptables: sizeof(struct xt_nfacct_match_info)) : 36
> 
> Trying to append nfacct related rules results in an unhelpful message.
> Although it is suggested to look for more information in dmesg, nothing
> can be found there.
> 
>     # iptables -A <chain> -m nfacct --nfacct-name <acct-object>
>     iptables: Invalid argument. Run `dmesg' for more information.
> 
> This patch fixes the memory misalignment by enforcing 8-byte alignment
> within the struct's first revision. This solution is often used in many
> other uapi netfilter headers.
> 
> Signed-off-by: Juliana Rodrigueiro <juliana.rodrigueiro@intra2net.com>

Thanks, this looks good.

Acked-by: Florian Westphal <fw@strlen.de>

Re: [PATCH v2] netfilter: nfacct: Fix alignment mismatch in xt_nfacct_match_info

[Pablo Neira Ayuso]

On Fri, Aug 16, 2019 at 05:02:22PM +0200, Juliana Rodrigueiro wrote:
> When running a 64-bit kernel with a 32-bit iptables binary, the size of
> the xt_nfacct_match_info struct diverges.
> 
>     kernel: sizeof(struct xt_nfacct_match_info) : 40
>     iptables: sizeof(struct xt_nfacct_match_info)) : 36
> 
> Trying to append nfacct related rules results in an unhelpful message.
> Although it is suggested to look for more information in dmesg, nothing
> can be found there.
> 
>     # iptables -A <chain> -m nfacct --nfacct-name <acct-object>
>     iptables: Invalid argument. Run `dmesg' for more information.
> 
> This patch fixes the memory misalignment by enforcing 8-byte alignment
> within the struct's first revision. This solution is often used in many
> other uapi netfilter headers.

Applied, thanks.

Please, send us the userspace chunk for iptables. Thanks again.