[PATCH] Fix ERROR target on CRIS

[PATCH] Fix ERROR target on CRIS

[Jesper Bengtsson]

This patch corrects a problem with adding ERROR targets on architectures that don't align data structues, e.g. CRIS.

Signed-off-by: Jesper Bengtsson <jesper.bengtsson@axis.com>
---
Index: linux-2.6/include/linux/netfilter_ipv4/ip_tables.h
===================================================================
RCS file: /usr/local/cvs/linux/os/linux-2.6/include/linux/netfilter_ipv4/ip_tables.h,v
retrieving revision 1.19
diff -b -u -p -r1.19 ip_tables.h
--- linux-2.6/include/linux/netfilter_ipv4/ip_tables.h 21 Aug 2008 08:40:42 -0000 1.19
+++ linux-2.6/include/linux/netfilter_ipv4/ip_tables.h 15 Oct 2008 12:55:31 -0000
@@ -259,7 +259,7 @@ struct ipt_standard
 struct ipt_error_target
 {
  struct ipt_entry_target target;
- char errorname[IPT_FUNCTION_MAXNAMELEN];
+ char errorname[IPT_TABLE_MAXNAMELEN];
 };

 struct ipt_error
Index: linux-2.6/net/ipv4/netfilter/ip_tables.c
===================================================================
RCS file: /usr/local/cvs/linux/os/linux-2.6/net/ipv4/netfilter/ip_tables.c,v
retrieving revision 1.29
diff -b -u -p -r1.29 ip_tables.c
--- linux-2.6/net/ipv4/netfilter/ip_tables.c 21 Aug 2008 08:41:02 -0000 1.29
+++ linux-2.6/net/ipv4/netfilter/ip_tables.c 15 Oct 2008 12:55:31 -0000
@@ -2184,7 +2184,7 @@ static struct xt_target ipt_standard_tar
 static struct xt_target ipt_error_target __read_mostly = {
  .name  = IPT_ERROR_TARGET,
  .target  = ipt_error,
- .targetsize = IPT_FUNCTION_MAXNAMELEN,
+ .targetsize = IPT_TABLE_MAXNAMELEN,
  .family  = AF_INET,
 };

Re: [PATCH] Fix ERROR target on CRIS

[Jan Engelhardt]

On Wednesday 2008-10-15 15:43, Jesper Bengtsson wrote:

>This patch corrects a problem with adding ERROR targets on architectures that don't align data structues, e.g. CRIS.
>
>Signed-off-by: Jesper Bengtsson <jesper.bengtsson@axis.com>
>---
>Index: linux-2.6/include/linux/netfilter_ipv4/ip_tables.h
>===================================================================
>RCS file: /usr/local/cvs/linux/os/linux-2.6/include/linux/netfilter_ipv4/ip_tables.h,v
>retrieving revision 1.19
>diff -b -u -p -r1.19 ip_tables.h
>--- linux-2.6/include/linux/netfilter_ipv4/ip_tables.h 21 Aug 2008 08:40:42 -0000 1.19
>+++ linux-2.6/include/linux/netfilter_ipv4/ip_tables.h 15 Oct 2008 12:55:31 -0000
>@@ -259,7 +259,7 @@ struct ipt_standard
> struct ipt_error_target
> {
>  struct ipt_entry_target target;
>- char errorname[IPT_FUNCTION_MAXNAMELEN];
>+ char errorname[IPT_TABLE_MAXNAMELEN];

Resolve the indirect macro - use XT_TABLE_MAXNAMELEN.
Fix up IPv6 too?
This looks not quite right to me though it is of course one way
to achieve to goal.. How about this?:

	char errorname[XT_ALIGN(XT_FUNCTION_MAXNAMELEN)]

is what I would suggest.

RE: [PATCH] Fix ERROR target on CRIS

[Jesper Bengtsson]

> >This patch corrects a problem with adding ERROR targets on
> architectures that don't align data structues, e.g. CRIS.
> >
> >Signed-off-by: Jesper Bengtsson <jesper.bengtsson@axis.com>
> >---
> >Index: linux-2.6/include/linux/netfilter_ipv4/ip_tables.h
> >===================================================================
> >RCS file:
> /usr/local/cvs/linux/os/linux-2.6/include/linux/netfilter_ipv4
> /ip_tables.h,v
> >retrieving revision 1.19
> >diff -b -u -p -r1.19 ip_tables.h
> >--- linux-2.6/include/linux/netfilter_ipv4/ip_tables.h 21
> Aug 2008 08:40:42 -0000 1.19
> >+++ linux-2.6/include/linux/netfilter_ipv4/ip_tables.h 15
> Oct 2008 12:55:31 -0000
> >@@ -259,7 +259,7 @@ struct ipt_standard
> > struct ipt_error_target
> > {
> >  struct ipt_entry_target target;
> >- char errorname[IPT_FUNCTION_MAXNAMELEN];
> >+ char errorname[IPT_TABLE_MAXNAMELEN];

First, my patch description was a bit incomplete. Sorry.
Here's a better description (I hope...).

The ipt_error_target structure is defined in both user space (iptables) and kernel space.
The problem is that the member 'errorname' has different length in the two definitions.
Iptables: char error[TABLE_MAXNAMELEN]; which is 32 bytes.
Kernel: char errorname[IPT_FUNCTION_MAXNAMELEN]; which is 30 bytes.

When trying to add an ERROR target, using iptables, the kernel will discard the request since the length doesn't match the kernel's error target. This problem doesn't show on an architecture that aligns data since the iptabels and kernel structures will have the same size.

I choose to change the kernel definition based on Patrick McHardy's suggestion:
http://marc.info/?l=netfilter-devel&m=122398633329014&w=2


> Resolve the indirect macro - use XT_TABLE_MAXNAMELEN.
Why not use the macro? The other structures defined in this file is using the macro. Shall those definitions also be changed?

> Fix up IPv6 too?
The corresponding ip6t_error_target structure definition is identical in iptables and kernel as far as I can tell.

> This looks not quite right to me though it is of course one way
> to achieve to goal.. How about this?:
>
>         char errorname[XT_ALIGN(XT_FUNCTION_MAXNAMELEN)]
The XT_ALIGN macro won't do any difference on the CRIS architecture since it's using 1 byte alignment.

Best regards,
Jesper

Re: [PATCH] Fix ERROR target on CRIS

[Patrick McHardy]

Jesper Bengtsson wrote:
> This patch corrects a problem with adding ERROR targets on architectures that don't align data structues, e.g. CRIS.
> 
> --- linux-2.6/include/linux/netfilter_ipv4/ip_tables.h 21 Aug 2008 08:40:42 -0000 1.19
> +++ linux-2.6/include/linux/netfilter_ipv4/ip_tables.h 15 Oct 2008 12:55:31 -0000
> @@ -259,7 +259,7 @@ struct ipt_standard
>  struct ipt_error_target
>  {
>   struct ipt_entry_target target;
> - char errorname[IPT_FUNCTION_MAXNAMELEN];
> + char errorname[IPT_TABLE_MAXNAMELEN];

Your mailer whitespace-corrupted the patch, please send as attachment.
Please also fix up ip6_tables and arp_tables.

Re: [PATCH] Fix ERROR target on CRIS

[Patrick McHardy]

Jan Engelhardt wrote:
>> struct ipt_error_target
>> {
>>  struct ipt_entry_target target;
>> - char errorname[IPT_FUNCTION_MAXNAMELEN];
>> + char errorname[IPT_TABLE_MAXNAMELEN];
> 
> Resolve the indirect macro - use XT_TABLE_MAXNAMELEN.

Thats also fine of course.

> Fix up IPv6 too?
> This looks not quite right to me though it is of course one way
> to achieve to goal.. How about this?:
> 
> 	char errorname[XT_ALIGN(XT_FUNCTION_MAXNAMELEN)]
> 
> is what I would suggest.

That misses the point, CRIS doesn't require any alignment and
FUNCTION_MAXNAMELEN is wrong here.

RE: [PATCH] Fix ERROR target on CRIS

[Jan Engelhardt]

On Thursday 2008-10-16 07:28, Jesper Bengtsson wrote:
>
>The ipt_error_target structure is defined in both user space (iptables) 
>and kernel space. The problem is that the member 'errorname' has 
>different length in the two definitions. Iptables: char 
>error[TABLE_MAXNAMELEN]; which is 32 bytes. Kernel: char 
>errorname[IPT_FUNCTION_MAXNAMELEN]; which is 30 bytes.

Oh :/  I had assumed that userspace uses IPT_FUNCTION_MAXNAMELEN too, 
but *aligned it* to a boundary of at least 2, since there was a recent 
report that ARM also had strange alignments in a way such that alignment 
in userspace was different than alignment in the kernel:
http://marc.info/?l=netfilter-devel&m=122309437709848&w=2


But!

12:11 nuqneh:~/Coding/iptables > grep -r '\b'TABLE_MAXNAME .
./libiptc/libip4tc.c:#define TABLE_MAXNAMELEN   IPT_TABLE_MAXNAMELEN
./libiptc/libip6tc.c:#define TABLE_MAXNAMELEN   IP6T_TABLE_MAXNAMELEN

So TABLE_MAXNAMELEN seems to be the same as IPT_TABLE_MAXNAMELEN, so 
TABLE_MAXNAMELEN too is 30, is not it?


>> Resolve the indirect macro - use XT_TABLE_MAXNAMELEN.
>
>Why not use the macro?

I meant: use XT_TABLE_MAXNAMELEN over IPT_TABLE_MAXNAMELEN,
since the latter is just a redirect.