[PATCH (resend)][EBTABLES]: Fix alignment checks in ebt_among.ko module.

[PATCH (resend)][EBTABLES]: Fix alignment checks in ebt_among.ko module.

[Pavel Emelyanov]

I've sent this patch some days ago to Bart, but with no answer...

When trying to do

	# ebtables -A FORWARD --among-src 0:12:34:56:78:9a=192.168.0.10 -j ACCEPT

on x86_64 box the ebt_among->check() callback warns me that

	ebtables: among: wrong size: 1060 against expected 1056, rounded to 1056

Checking the ebtables sources, I found that the alignment is done
differently in the tool and the kernel. Tool makes it like this:

	EBT_ALIGN(sizeof(struct ebt_among_info)) + X

while the kernel module like this:

	EBT_ALIGN(sizeof(struct ebt_among_info) + X)

So the suggested fix is to move the alignment in the kernel. After
the fix the rule is added and appears in the ebtables -L output.

Originally developed by Evgeny Kravtsunov.

Signed-off-by: Evgeny Kravtsunov <emkravts@openvz.org>
Signed-off-by: Pavel Emelyanov <xemul@openvz.org>

---

diff --git a/net/bridge/netfilter/ebt_among.c b/net/bridge/netfilter/ebt_among.c
index 70b6dca..349a543 100644
--- a/net/bridge/netfilter/ebt_among.c
+++ b/net/bridge/netfilter/ebt_among.c
@@ -182,7 +182,7 @@ static int ebt_among_check(const char *tablename, unsigned int hookmask,
 			   unsigned int datalen)
 {
 	const struct ebt_among_info *info = data;
-	int expected_length = sizeof(struct ebt_among_info);
+	int expected_length = EBT_ALIGN(sizeof(struct ebt_among_info));
 	const struct ebt_mac_wormhash *wh_dst, *wh_src;
 	int err;
 
@@ -191,7 +191,7 @@ static int ebt_among_check(const char *tablename, unsigned int hookmask,
 	expected_length += ebt_mac_wormhash_size(wh_dst);
 	expected_length += ebt_mac_wormhash_size(wh_src);
 
-	if (datalen != EBT_ALIGN(expected_length)) {
+	if (datalen != expected_length) {
 		printk(KERN_WARNING
 		       "ebtables: among: wrong size: %d "
 		       "against expected %d, rounded to %Zd\n",

Re: [PATCH (resend)][EBTABLES]: Fix alignment checks in ebt_among.ko module.

[Patrick McHardy]

Pavel Emelyanov wrote:
> I've sent this patch some days ago to Bart, but with no answer...
> 
> When trying to do
> 
> 	# ebtables -A FORWARD --among-src 0:12:34:56:78:9a=192.168.0.10 -j ACCEPT
> 
> on x86_64 box the ebt_among->check() callback warns me that
> 
> 	ebtables: among: wrong size: 1060 against expected 1056, rounded to 1056
> 
> Checking the ebtables sources, I found that the alignment is done
> differently in the tool and the kernel. Tool makes it like this:
> 
> 	EBT_ALIGN(sizeof(struct ebt_among_info)) + X
> 
> while the kernel module like this:
> 
> 	EBT_ALIGN(sizeof(struct ebt_among_info) + X)
> 
> So the suggested fix is to move the alignment in the kernel. After
> the fix the rule is added and appears in the ebtables -L output.


It seems the kernel is correct and userspace is doing it
wrong, so I think userspace should be fixed instead.
The problem with your patch is that is causes misalignment
for following structures that contain u64 members.

Re: [PATCH (resend)][EBTABLES]: Fix alignment checks in ebt_among.ko module.

[Jan Engelhardt]

On Feb 29 2008 13:48, Patrick McHardy wrote:
> Pavel Emelyanov wrote:
>> I've sent this patch some days ago to Bart, but with no answer...
>> 
>> When trying to do
>> 
>>  # ebtables -A FORWARD --among-src 0:12:34:56:78:9a=192.168.0.10 -j ACCEPT
>> 
>> on x86_64 box the ebt_among->check() callback warns me that
>> 
>>  ebtables: among: wrong size: 1060 against expected 1056, rounded to 1056
>> [...]
>
> It seems the kernel is correct and userspace is doing it
> wrong, so I think userspace should be fixed instead.
> The problem with your patch is that is causes misalignment
> for following structures that contain u64 members.

ebt_among is a true mess! It hinders my efforts to port ebtables
to xtables, but I could of course use a hack and will likely
do just that.

Re: [PATCH (resend)][EBTABLES]: Fix alignment checks in ebt_among.ko module.

[Bart De Schuymer]

Op vr, 29-02-2008 te 16:40 +0100, schreef Jan Engelhardt:
> > It seems the kernel is correct and userspace is doing it
> > wrong, so I think userspace should be fixed instead.
> > The problem with your patch is that is causes misalignment
> > for following structures that contain u64 members.
> 
> ebt_among is a true mess! It hinders my efforts to port ebtables
> to xtables, but I could of course use a hack and will likely
> do just that.

Ebtables' dynamic memory allocation scheme is probably not portable to
iptables. I agree the fix is in userspace. Anyway, I think the author of
that module did a great job and provided a very useful and often-used
functionality.

Bart

Re: [PATCH (resend)][EBTABLES]: Fix alignment checks in ebt_among.ko module.

[Jan Engelhardt]

On Feb 29 2008 18:23, Bart De Schuymer wrote:
>Op vr, 29-02-2008 te 16:40 +0100, schreef Jan Engelhardt:
>> > It seems the kernel is correct and userspace is doing it
>> > wrong, so I think userspace should be fixed instead.
>> > The problem with your patch is that is causes misalignment
>> > for following structures that contain u64 members.
>> 
>> ebt_among is a true mess! It hinders my efforts to port ebtables
>> to xtables, but I could of course use a hack and will likely
>> do just that.
>
>Ebtables' dynamic memory allocation scheme is probably not portable to
>iptables. I agree the fix is in userspace. Anyway, I think the author of
>that module did a great job and provided a very useful and often-used
>functionality.

Actually yes, it's kind of a weird mixup of TLV and static blocks :p

Re: [PATCH (resend)][EBTABLES]: Fix alignment checks in ebt_among.ko module.

[Bart De Schuymer]

[-- Attachment #1: Type: text/plain, Size: 940 bytes --]

Op vr, 29-02-2008 te 13:48 +0100, schreef Patrick McHardy:
> Pavel Emelyanov wrote:
> > I've sent this patch some days ago to Bart, but with no answer...

Sorry about that.

> > 
> > When trying to do
> > 
> > 	# ebtables -A FORWARD --among-src 0:12:34:56:78:9a=192.168.0.10 -j ACCEPT
> > 
> > on x86_64 box the ebt_among->check() callback warns me that
> > 
> > 	ebtables: among: wrong size: 1060 against expected 1056, rounded to 1056
> > 
> > So the suggested fix is to move the alignment in the kernel. After
> > the fix the rule is added and appears in the ebtables -L output.
> 
> 
> It seems the kernel is correct and userspace is doing it
> wrong, so I think userspace should be fixed instead.
> The problem with your patch is that is causes misalignment
> for following structures that contain u64 members.

Pavel, please try the attached userspace patch to see if it fixes the
problem (and let me know the result).

cheers,
Bart


[-- Attachment #2: among64.diff --]
[-- Type: text/x-patch, Size: 1961 bytes --]

--- ebtables-v2.0.8-2/extensions/ebt_among.c.old	2008-03-02 16:08:13.000000000 +0100
+++ ebtables-v2.0.8-2/extensions/ebt_among.c	2008-03-02 16:06:55.000000000 +0100
@@ -68,6 +68,7 @@ static void print_help()
 " 00:00:00:fa:eb:fe=153.19.120.250,00:00:00:fa:eb:fe=192.168.0.1\n"
 	);
 }
+static int old_size;
 
 static void init(struct ebt_entry_match *match)
 {
@@ -75,6 +76,7 @@ static void init(struct ebt_entry_match 
 	    (struct ebt_among_info *) match->data;
 
 	memset(amonginfo, 0, sizeof(struct ebt_among_info));
+	old_size = sizeof(struct ebt_among_info);
 }
 
 static struct ebt_mac_wormhash *new_wormhash(int n)
@@ -308,7 +310,7 @@ static int parse(int c, char **argv, int
 	    (struct ebt_among_info *) (*match)->data;
 	struct ebt_mac_wormhash *wh;
 	struct ebt_entry_match *h;
-	int new_size, old_size;
+	int new_size;
 	long flen;
 	int fd;
 
@@ -354,21 +356,23 @@ static int parse(int c, char **argv, int
 		if (ebt_errormsg[0] != '\0')
 			break;
 
-		old_size = sizeof(struct ebt_entry_match) + (**match).match_size;
-		h = malloc((new_size = old_size + ebt_mac_wormhash_size(wh)));
+		new_size = old_size+ebt_mac_wormhash_size(wh);
+		h = malloc(sizeof(struct ebt_entry_match)+EBT_ALIGN(new_size));
 		if (!h)
 			ebt_print_memory();
-		memcpy(h, *match, old_size);
-		memcpy((char *) h + old_size, wh, ebt_mac_wormhash_size(wh));
-		h->match_size = new_size - sizeof(struct ebt_entry_match);
+		memcpy(h, *match, old_size+sizeof(struct ebt_entry_match));
+		memcpy((char *)h+old_size+sizeof(struct ebt_entry_match), wh,
+		       ebt_mac_wormhash_size(wh));
+		h->match_size = EBT_ALIGN(new_size);
 		info = (struct ebt_among_info *) h->data;
 		if (c == AMONG_DST) {
 			info->wh_dst_ofs =
-			    old_size - sizeof(struct ebt_entry_match);
+			    old_size;
 		} else {
 			info->wh_src_ofs =
-			    old_size - sizeof(struct ebt_entry_match);
+			    old_size;
 		}
+		old_size = new_size;
 		free(*match);
 		*match = h;
 		free(wh);

Re: [PATCH (resend)][EBTABLES]: Fix alignment checks in ebt_among.ko module.

[Pavel Emelyanov]

Bart De Schuymer wrote:
> Op vr, 29-02-2008 te 13:48 +0100, schreef Patrick McHardy:
>> Pavel Emelyanov wrote:
>>> I've sent this patch some days ago to Bart, but with no answer...
> 
> Sorry about that.
> 
>>> When trying to do
>>>
>>> 	# ebtables -A FORWARD --among-src 0:12:34:56:78:9a=192.168.0.10 -j ACCEPT
>>>
>>> on x86_64 box the ebt_among->check() callback warns me that
>>>
>>> 	ebtables: among: wrong size: 1060 against expected 1056, rounded to 1056
>>>
>>> So the suggested fix is to move the alignment in the kernel. After
>>> the fix the rule is added and appears in the ebtables -L output.
>>
>> It seems the kernel is correct and userspace is doing it
>> wrong, so I think userspace should be fixed instead.
>> The problem with your patch is that is causes misalignment
>> for following structures that contain u64 members.
> 
> Pavel, please try the attached userspace patch to see if it fixes the
> problem (and let me know the result).

Tested - works OK. Thanks.

> cheers,
> Bart
> 
>