Re: different packing of structs in kernel vs. userland ?
On Tue, 16 Jul 2002, David Taylor wrote: Bah, ignore me, it appears you've already admitted your post was rather less than clear :) -- David Taylor [EMAIL PROTECTED] "The future just ain't what it used to be" To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: different packing of structs in kernel vs. userland ?
On Mon, 15 Jul 2002, Terry Lambert wrote: > Thomas Moestl wrote: > > > He's making the valid point that for: > > > > > > struct foo *fee; > > > > > > It's possible that: > > > > > > sizeof(struct foo) != (((char *)&fee[1]) - ((char *)&fee[0])) > > > > No, I do not. In fact, the opposite: > > > > sizeof(struct foo) = (((char *)&fee[1]) - ((char *)&fee[0])) > > > > _must_ always be true > Reread my second to last paragraph. I'm saying the same thing > that you are. How can you possibly be saying the same thing, when you are saying the exact opposite (You: A != B may be true, Thomas: A == B must be true)? You're saying: sizeof(struct foo) != (((char *)&fee[1]) - ((char *)&fee[0])) which would imply that end-padding is not included in sizeof(struct foo), when it must be, otherwise malloc(n * sizeof(struct foo)) would not allocate enough memory for an array of n elements of struct foo. -- David Taylor [EMAIL PROTECTED] "The future just ain't what it used to be" To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: different packing of structs in kernel vs. userland ?
Richard Tobin wrote: > It is not a valid point that it's possible that > > sizeof(struct foo) != (((char *)&fee[1]) - ((char *)&fee[0])) > > because it isn't possible. It must be the case that > > sizeof(struct foo) == (((char *)&fee[1]) - ((char *)&fee[0])) > > If that's what you meant, you seem to be saying the opposite. I guess I should have been clearer; reading it again, it's clearly a double entendre on my part. Sorry for the confusion it caused. > > and that inter-structure padding depends on ordering of elements > > Yes, though it isn't "inter-structure" padding, it's padding that > is part of the structure. Yes, because the compiler can't know that a pointer will not have arithmatic done on it, because it can't know whether the element it points to is the first in a series, or only a single element. This is the source of the confusion in the size test that was broken in the case that started this thread. > > (for a good example of this, see the struct direct name element > > reference macro, which is also padding independent). > > Not sure which macro you mean here, since there are several variants > of it which work in different ways. The one in dirent.h (I'm looking > at 4.6 here) subtracts from sizeof, while the one in ufs/dir.h uses > &(0->d_name) which is equivalent to offsetof. This is the one (n.b: there didn't used to be an "offsetof"). > > Basically, end-padding happens because arrays of structures need to > > have their first element properly aligned, so there is a pad added > > after each element to ensure that the following element starts on > > an alignment boundary. > > The point is that there isn't a pad *after* each element. The pad is > part of the element, and is there regardless of whether the structure > is in an array. Again, if that's what you meant you seem to be saying > the opposite! I think I just used ambiguous language in the initial part, and that threw off everything after it. Like the SNL joke "You can't put too much water in a nuclear reactor". -- Terry To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: different packing of structs in kernel vs. userland ?
> If everyone could read the text past my example of bad math, so > that they could know it was an intentional example of bad math, > live would be beautiful. 8-). I did read past it, and I just read it again, and I can't make it come out any way other than it did the first time. You said: > He's making the valid point that for: > > struct foo *fee; > > It's possible that: > > sizeof(struct foo) != (((char *)&fee[1]) - ((char *)&fee[0])) > > because of end-padding, which is not accounted for in arrays, It is not a valid point that it's possible that sizeof(struct foo) != (((char *)&fee[1]) - ((char *)&fee[0])) because it isn't possible. It must be the case that sizeof(struct foo) == (((char *)&fee[1]) - ((char *)&fee[0])) If that's what you meant, you seem to be saying the opposite. > and that inter-structure padding depends on ordering of elements Yes, though it isn't "inter-structure" padding, it's padding that is part of the structure. > (for a good example of this, see the struct direct name element > reference macro, which is also padding independent). Not sure which macro you mean here, since there are several variants of it which work in different ways. The one in dirent.h (I'm looking at 4.6 here) subtracts from sizeof, while the one in ufs/dir.h uses &(0->d_name) which is equivalent to offsetof. > Basically, end-padding happens because arrays of structures need to > have their first element properly aligned, so there is a pad added > after each element to ensure that the following element starts on > an alignment boundary. The point is that there isn't a pad *after* each element. The pad is part of the element, and is there regardless of whether the structure is in an array. Again, if that's what you meant you seem to be saying the opposite! -- Richard To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: different packing of structs in kernel vs. userland ?
Richard Tobin wrote: > Er, no, that's not right. Otherwise [ ... ] If everyone could read the text past my example of bad math, so that they could know it was an intentional example of bad math, live would be beautiful. 8-). -- Terry To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: different packing of structs in kernel vs. userland ?
Peter Edwards wrote: > > He's making the valid point that for: > > > > struct foo *fee; > > > > It's possible that: > > > > sizeof(struct foo) != (((char *)&fee[1]) - ((char *)&fee[0])) > > Wouldn't that mean > > .. struct X *xarr = malloc(sizeof (struct X) * arrayLen); > > wouldn't produce a useable array of struct X of length arrayLen? > That can't be right. No, it doesn't mean that, because there *is* end padding in the size calculation. That's kind of the whole point of this thread. In the directory entry example, the last element is a d_name[1] that's an overlay array that's much longer than it appears (it is always at least 4 bytes). This is because it's meant to work with other than C99 (which permits an array with a declared size of 0). It's necessary for the code to know the offset of the start of that elemenet from the start of the structure, and that offset is *NOT* "sizeof(struct) - 1", as you would naievely believe, as if there were no end padding, because the total size of the structure is not always going to be evenly divisible by the number of bytes between alignment boundaries. Does this make more sense?!? -- Terry To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: different packing of structs in kernel vs. userland ?
Thomas Moestl wrote: > > He's making the valid point that for: > > > > struct foo *fee; > > > > It's possible that: > > > > sizeof(struct foo) != (((char *)&fee[1]) - ((char *)&fee[0])) > > No, I do not. In fact, the opposite: > > sizeof(struct foo) = (((char *)&fee[1]) - ((char *)&fee[0])) > > _must_ always be true, since it is legal to compute the size of > storage needed for an n-element array of struct foo by using > (sizeof(struct foo) * n). > > My point was that, because of the above, any padding that might be > required between the first and last member of two struct foo's > immediately following each other must be _included_ in struct foo, > after the last element. Reread my second to last paragraph. I'm saying the same thing that you are. In my third to last paragrap, I pointed to an example for directory entries that ensures end-pad independence (I wrote that code). -- Terry To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: different packing of structs in kernel vs. userland ?
Thomas Moestl <[EMAIL PROTECTED]> writes: > Oh, right, that's related: the kernel checks for a minimum size of the > passed data on two occasions, first in sooptcopyin(), and then again > in check_ipfw_struct(). > It the size to be at least sizeof(struct ip_fw), however for > structures containing just one action (like the one for the command > above) this is again too much in the 64-bit case because of the > padding. Can you please try the attached patch (against the CVS > version)? Yes, this version works. %%% bowie# ipfw show 00100 0 0 allow ip from me to 192.168.3.1 00200 5484 allow udp from me to 192.168.3.13 00300 0 0 allow tcp from me to 192.168.3.0/24 established 00400 0 0 deny ip from me to 192.168.3.0/24 00500 9734 allow ip from any to any 65535 0 0 deny ip from any to any %%% Best regards, Mike Barcroft To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: different packing of structs in kernel vs. userland ?
ok, convincing explaination, thanks! I will review the code and try to implement a proper fix, not the workaround that I put in. cheers luigi To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: different packing of structs in kernel vs. userland ?
> struct foo *fee; > > It's possible that: > > sizeof(struct foo) != (((char *)&fee[1]) - ((char *)&fee[0])) > > because of end-padding, which is not accounted for in arrays, Er, no, that's not right. Otherwise fee = malloc(n * sizeof(struct foo)) wouldn't work. C89 says: There may also be unnamed padding at the end of a structure or union, as necessary to achieve the proper alignment were the structure or union to be an element of an array. And: the result [of sizeof] is the total number of bytes in such an object, including internal and trailing padding. So if a struct needs padding in an array, it has it even when it isn't in an array. -- Richard To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: different packing of structs in kernel vs. userland ?
Hi, > He's making the valid point that for: > > struct foo *fee; > > It's possible that: > > sizeof(struct foo) != (((char *)&fee[1]) - ((char *)&fee[0])) Wouldn't that mean .. struct X *xarr = malloc(sizeof (struct X) * arrayLen); wouldn't produce a useable array of struct X of length arrayLen? That can't be right. -- Peter Edwards. To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: different packing of structs in kernel vs. userland ?
On Mon, 2002/07/15 at 04:24:33 -0700, Terry Lambert wrote:
> Luigi Rizzo wrote:
> > sorry but all this just does not make sense to me.
> >
> > sizeof(foo) should give the same result irrespective of
> > where you use it.
> >
> > Perhaps the best thing would be to put a
> >
> > printf("struct ip_fw has size %d\n", sizeof(struct ip_fw));
> >
> > both in ipfw2.c and somewhere in ip_fw2.c and see if there is
> > a mismatch between the two numbers.
>
> I have to assume that what didn't make sense was that his patch
> worked? 8-).
>
> He's making the valid point that for:
>
> struct foo *fee;
>
> It's possible that:
>
> sizeof(struct foo) != (((char *)&fee[1]) - ((char *)&fee[0]))
No, I do not. In fact, the opposite:
sizeof(struct foo) = (((char *)&fee[1]) - ((char *)&fee[0]))
_must_ always be true, since it is legal to compute the size of
storage needed for an n-element array of struct foo by using
(sizeof(struct foo) * n).
My point was that, because of the above, any padding that might be
required between the first and last member of two struct foo's
immediately following each other must be _included_ in struct foo,
after the last element.
- thomas
--
Thomas Moestl <[EMAIL PROTECTED]> http://www.tu-bs.de/~y0015675/
<[EMAIL PROTECTED]> http://people.FreeBSD.org/~tmm/
PGP fingerprint: 1C97 A604 2BD0 E492 51D0 9C0F 1FE6 4F1D 419C 776C
To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message
Re: different packing of structs in kernel vs. userland ?
On Mon, 2002/07/15 at 04:00:08 -0700, Luigi Rizzo wrote:
> sorry but all this just does not make sense to me.
>
> sizeof(foo) should give the same result irrespective of
> where you use it.
OK, let me rephrase it: as I explained before, struct ip_fw has padding
after 'cmd' (the last member) to ensure that arrays can be built from
it safely, so that the first member will always be properly
aligned.
Since the first members must/should be aligned on an 8-bit boundary on
64-bit platforms, this means that sizeof(struct ip_fw) must be a
multiple of 8, the size of the padding is 4 bytes (unless the
situation is changed by reordering structure members). This can easily
be checked on a 64-bit platform. The following program fragment:
struct ip_fw f;
printf("sizeof(ip_fw) = %d\n", (int)sizeof(f));
printf("offsetof(ip_fw, cmd) = %d\n", (int)offsetof(struct ip_fw, cmd));
printf("sizeof(ip_fw.cmd) = %d\n", (int)sizeof(f.cmd));
Produces this output on sparc64:
sizeof(ip_fw) = 56
offsetof(ip_fw, cmd) = 48
sizeof(ip_fw.cmd) = 4
This illustrates that indeed, padding is appended after 'cmd'.
In the (userland) ipfw2.c, you basically do the following:
ipfw_insn *dst; /* sizeof(ipfw_insn) = 4 */
dst = (ipfw_insn *)rule->cmd;
/* Write n instructions and increase dst accordingly. */
rule->cmd_len = (u_int32_t *)dst - (u_int32_t *)(rule->cmd);
i = (void *)dst - (void *)rule;
if (getsockopt(s, IPPROTO_IP, IP_FW_ADD, rule, &i) == -1)
err(EX_UNAVAILABLE, "getsockopt(%s)", "IP_FW_ADD");
Let's consider the case where only one instruction was added. In this
case, dst was incremented once and points directly after cmd, so i is
52 on a 64-bit platform.
However, sizeof(struct ip_fw) is 56 because the aformentioned 4 bytes
of padding following 'cmd', so i < sizeof(struct ip_fw). This explains
why rules with just one instruction would not work properly in this
case with just my first patch.
Likewise, when adding more rules, the second one will be added to the
memory location directly following 'cmd'. If padding is present, the
second instruction will write into it. The size of the total structure
will thus not be properly computed by the old RULESIZE macro:
#define RULESIZE(rule) (sizeof(struct ip_fw) + \
((struct ip_fw *)(rule))->cmd_len * 4 - 4)
The '- 4' is meant to subtract the size of the cmd, which is accounted
for in cmd_len. Still, you are counting the padding twice, once in the
sizeof() and once in cmd_len.
So, sizeof(struct ip_fw) is no different between userland and kernel,
but the problem is that you don't use sizeof(struct ip_fw) in userland
to compute the sizes (but pointer arithmetic), but you do use it for
the checks in the kernel.
- thomas
--
Thomas Moestl <[EMAIL PROTECTED]> http://www.tu-bs.de/~y0015675/
<[EMAIL PROTECTED]> http://people.FreeBSD.org/~tmm/
PGP fingerprint: 1C97 A604 2BD0 E492 51D0 9C0F 1FE6 4F1D 419C 776C
To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message
Re: different packing of structs in kernel vs. userland ?
Luigi Rizzo wrote:
> sorry but all this just does not make sense to me.
>
> sizeof(foo) should give the same result irrespective of
> where you use it.
>
> Perhaps the best thing would be to put a
>
> printf("struct ip_fw has size %d\n", sizeof(struct ip_fw));
>
> both in ipfw2.c and somewhere in ip_fw2.c and see if there is
> a mismatch between the two numbers.
I have to assume that what didn't make sense was that his patch
worked? 8-).
He's making the valid point that for:
struct foo *fee;
It's possible that:
sizeof(struct foo) != (((char *)&fee[1]) - ((char *)&fee[0]))
because of end-padding, which is not accounted for in arrays,
and that inter-structure padding depends on ordering of elements
(for a good example of this, see the struct direct name element
reference macro, which is also padding independent).
Basically, end-padding happens because arrays of structures need to
have their first element properly aligned, so there is a pad added
after each element to ensure that the following element starts on
an alignment boundary.
I still say that on 486 and higher, the "disallow unaligned access"
bit in the processor control register should be enabled, so your
kernel will panic if you try this. 8-).
-- Terry
To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message
Re: different packing of structs in kernel vs. userland ?
sorry but all this just does not make sense to me.
sizeof(foo) should give the same result irrespective of
where you use it.
Perhaps the best thing would be to put a
printf("struct ip_fw has size %d\n", sizeof(struct ip_fw));
both in ipfw2.c and somewhere in ip_fw2.c and see if there is
a mismatch between the two numbers.
cheers
luigi
On Mon, Jul 15, 2002 at 12:51:58PM +0200, Thomas Moestl wrote:
> On Sun, 2002/07/14 at 23:08:21 -0400, Mike Barcroft wrote:
> > Thomas Moestl <[EMAIL PROTECTED]> writes:
> > > (Disclaimer: my solution below is untested, so it may all be bogus)
> >
> > As request, here are the test results.
> >
> > Most rules work, except my final one:
> > %%%
> > bowie# ipfw add allow all from any to any
> > ipfw: getsockopt(IP_FW_ADD): Invalid argument
> > %%%
>
> Oh, right, that's related: the kernel checks for a minimum size of the
> passed data on two occasions, first in sooptcopyin(), and then again
> in check_ipfw_struct().
> It the size to be at least sizeof(struct ip_fw), however for
> structures containing just one action (like the one for the command
> above) this is again too much in the 64-bit case because of the
> padding. Can you please try the attached patch (against the CVS
> version)?
>
> - thomas
>
> --
> Thomas Moestl <[EMAIL PROTECTED]> http://www.tu-bs.de/~y0015675/
> <[EMAIL PROTECTED]> http://people.FreeBSD.org/~tmm/
> PGP fingerprint: 1C97 A604 2BD0 E492 51D0 9C0F 1FE6 4F1D 419C 776C
>
> Index: ip_fw.h
> ===
> RCS file: /home/ncvs/src/sys/netinet/ip_fw.h,v
> retrieving revision 1.71
> diff -u -r1.71 ip_fw.h
> --- ip_fw.h 8 Jul 2002 22:39:19 - 1.71
> +++ ip_fw.h 15 Jul 2002 10:48:19 -
> @@ -294,8 +294,9 @@
> #define ACTION_PTR(rule) \
> (ipfw_insn *)( (u_int32_t *)((rule)->cmd) + ((rule)->act_ofs) )
>
> -#define RULESIZE(rule) (sizeof(struct ip_fw) + \
> - ((struct ip_fw *)(rule))->cmd_len * 4 - 4)
> +#define RULESIZE_FROMLEN(len) (offsetof(struct ip_fw, cmd) + (len) * 4)
> +#define RULESIZE(rule) RULESIZE_FROMLEN(((struct ip_fw *)(rule))->cmd_len)
> +#define RULESIZE_MINRULESIZE_FROMLEN(1)
>
> /*
> * This structure is used as a flow mask and a flow id for various
> Index: ip_fw2.c
> ===
> RCS file: /home/ncvs/src/sys/netinet/ip_fw2.c,v
> retrieving revision 1.4
> diff -u -r1.4 ip_fw2.c
> --- ip_fw2.c 8 Jul 2002 22:46:01 - 1.4
> +++ ip_fw2.c 15 Jul 2002 10:38:09 -
> @@ -2142,7 +2142,7 @@
> int have_action=0;
> ipfw_insn *cmd;
>
> - if (size < sizeof(*rule)) {
> + if (size < RULESIZE_MIN) {
> printf("ipfw: rule too short\n");
> return (EINVAL);
> }
> @@ -2428,7 +2428,7 @@
> case IP_FW_ADD:
> rule = (struct ip_fw *)rule_buf; /* XXX do a malloc */
> error = sooptcopyin(sopt, rule, sizeof(rule_buf),
> - sizeof(struct ip_fw) );
> + RULESIZE_MIN);
> size = sopt->sopt_valsize;
> if (error || (error = check_ipfw_struct(rule, size)))
> break;
To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message
Re: different packing of structs in kernel vs. userland ?
On Sun, 2002/07/14 at 23:08:21 -0400, Mike Barcroft wrote:
> Thomas Moestl <[EMAIL PROTECTED]> writes:
> > (Disclaimer: my solution below is untested, so it may all be bogus)
>
> As request, here are the test results.
>
> Most rules work, except my final one:
> %%%
> bowie# ipfw add allow all from any to any
> ipfw: getsockopt(IP_FW_ADD): Invalid argument
> %%%
Oh, right, that's related: the kernel checks for a minimum size of the
passed data on two occasions, first in sooptcopyin(), and then again
in check_ipfw_struct().
It the size to be at least sizeof(struct ip_fw), however for
structures containing just one action (like the one for the command
above) this is again too much in the 64-bit case because of the
padding. Can you please try the attached patch (against the CVS
version)?
- thomas
--
Thomas Moestl <[EMAIL PROTECTED]> http://www.tu-bs.de/~y0015675/
<[EMAIL PROTECTED]> http://people.FreeBSD.org/~tmm/
PGP fingerprint: 1C97 A604 2BD0 E492 51D0 9C0F 1FE6 4F1D 419C 776C
Index: ip_fw.h
===
RCS file: /home/ncvs/src/sys/netinet/ip_fw.h,v
retrieving revision 1.71
diff -u -r1.71 ip_fw.h
--- ip_fw.h 8 Jul 2002 22:39:19 - 1.71
+++ ip_fw.h 15 Jul 2002 10:48:19 -
@@ -294,8 +294,9 @@
#define ACTION_PTR(rule) \
(ipfw_insn *)( (u_int32_t *)((rule)->cmd) + ((rule)->act_ofs) )
-#define RULESIZE(rule) (sizeof(struct ip_fw) + \
- ((struct ip_fw *)(rule))->cmd_len * 4 - 4)
+#defineRULESIZE_FROMLEN(len) (offsetof(struct ip_fw, cmd) + (len) * 4)
+#defineRULESIZE(rule) RULESIZE_FROMLEN(((struct ip_fw *)(rule))->cmd_len)
+#defineRULESIZE_MINRULESIZE_FROMLEN(1)
/*
* This structure is used as a flow mask and a flow id for various
Index: ip_fw2.c
===
RCS file: /home/ncvs/src/sys/netinet/ip_fw2.c,v
retrieving revision 1.4
diff -u -r1.4 ip_fw2.c
--- ip_fw2.c8 Jul 2002 22:46:01 - 1.4
+++ ip_fw2.c15 Jul 2002 10:38:09 -
@@ -2142,7 +2142,7 @@
int have_action=0;
ipfw_insn *cmd;
- if (size < sizeof(*rule)) {
+ if (size < RULESIZE_MIN) {
printf("ipfw: rule too short\n");
return (EINVAL);
}
@@ -2428,7 +2428,7 @@
case IP_FW_ADD:
rule = (struct ip_fw *)rule_buf; /* XXX do a malloc */
error = sooptcopyin(sopt, rule, sizeof(rule_buf),
- sizeof(struct ip_fw) );
+ RULESIZE_MIN);
size = sopt->sopt_valsize;
if (error || (error = check_ipfw_struct(rule, size)))
break;
To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message
Re: different packing of structs in kernel vs. userland ?
Thomas Moestl <[EMAIL PROTECTED]> writes: > (Disclaimer: my solution below is untested, so it may all be bogus) As request, here are the test results. Most rules work, except my final one: %%% bowie# ipfw add allow all from any to any ipfw: getsockopt(IP_FW_ADD): Invalid argument %%% Here's a list of rules that applied okay: %%% ipfw bowie# ipfw show 00100 0 0 allow ip from me to 192.168.3.1 00200 2168 allow udp from me to 192.168.3.13 00300 0 0 allow tcp from me to 192.168.3.0/24 established 00400 0 0 deny ip from me to 192.168.3.0/24 65535 3219 deny ip from any to any %%% Best regards, Mike Barcroft To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: different packing of structs in kernel vs. userland ?
On Sun, 2002/07/14 at 13:43:37 -0700, Luigi Rizzo wrote: > [i am deliberately not trimming the email in case someone wants to > look at the context] > > i am a bit dubious about your explaination -- it also does not > explain why the person reporting this problem "fixed" that > by swapping "timestamp" and "next_rule" in the structure It does - doing so removes the need for padding before 'next_rule', because it is properly aligned then. 'timestamp' and 'cmd' are both 4 bytes in size and immediately follow each other, so the total structure size is a multiple of 8 (48 bytes). Because of that, no padding after 'cmd' is required, and the effect is gone. - thomas -- Thomas Moestl <[EMAIL PROTECTED]> http://www.tu-bs.de/~y0015675/ <[EMAIL PROTECTED]> http://people.FreeBSD.org/~tmm/ PGP fingerprint: 1C97 A604 2BD0 E492 51D0 9C0F 1FE6 4F1D 419C 776C To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: different packing of structs in kernel vs. userland ?
[i am deliberately not trimming the email in case someone wants to
look at the context]
i am a bit dubious about your explaination -- it also does not
explain why the person reporting this problem "fixed" that
by swapping "timestamp" and "next_rule" in the structure
cheers
luigi
On Sun, Jul 14, 2002 at 10:36:42PM +0200, Thomas Moestl wrote:
> On Sun, 2002/07/14 at 01:18:10 -0700, Luigi Rizzo wrote:
> > Hi,
> > the following message seems to suggest that the compiler
> > (the way it is invoked) packs structures differently
> > when building the kernel and userland.
> >
> > The stize of the structure in question is computed
> > by both kernel and userland app using sizeof(),
> > so there is no assumption on the size of its members,
> > so i believe the only possibility of a mismatch is
> > the one above.
> >
> > Any ideas ?
>
> (Disclaimer: my solution below is untested, so it may all be bogus)
>
> No, you are not accounting for "external" structure padding. Take a
> look:
>
> struct ip_fw {
> struct ip_fw *next; /* linked list of rules */
> u_int16_t act_ofs;/* offset of action in 32-bit units */
> u_int16_t cmd_len;/* # of 32-bit words in cmd */
> u_int16_t rulenum;/* rule number */
> u_int16_t _pad; /* padding */
>
> /* These fields are present in all rules. */
> u_int64_t pcnt; /* Packet counter */
> u_int64_t bcnt; /* Byte counter */
> u_int32_t timestamp; /* tv_sec of last match */
>
> struct ip_fw *next_rule; /* ptr to next rule */
>
> ipfw_insn cmd[1]; /* storage for commands */
> };
>
> On a 64-bit architecture, pointers are obviously 8 bytes in size;
> structure members must or should be on natural borders, depending on
> the architecture.
> So, next_rule will not be on a natural border; 4 bytes of padding will
> be inserted before it. With that, the total structure size would be
> 52.
> The compiler must account for the fact that an array of struct ip_fws
> may be used. For obvious reasons, it can not just insert extra padding
> in the array case; instead, the structure size must be chosen so that
> in this situation, the first member will be on a natural border.
> This results in an extra 4 bytes of "external" padding at the end,
> after the member 'cmd'.
> The macro you use to compute the size in the kernel is:
>
> #define RULESIZE(rule) (sizeof(struct ip_fw) + \
> ((struct ip_fw *)(rule))->cmd_len * 4 - 4)
>
> In the userland code, you start at &foo.cmd and append data
> directly. This means that the padding will also be used to store
> data, so the '- 4' (= sizeof(foo.cmd)) will not always be enough. The
> following definition of RULESIZE (untested) should fix this:
>
> #define RULESIZE(rule) (offsetof(struct ip_fw, cmd) + \
> ((struct ip_fw *)(rule))->cmd_len * 4)
>
> It also removes the explicit 4 for sizeof(ipfw_insn).
>
> - thomas
>
> --
> Thomas Moestl <[EMAIL PROTECTED]> http://www.tu-bs.de/~y0015675/
> <[EMAIL PROTECTED]> http://people.FreeBSD.org/~tmm/
> PGP fingerprint: 1C97 A604 2BD0 E492 51D0 9C0F 1FE6 4F1D 419C 776C
To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message
Re: different packing of structs in kernel vs. userland ?
On Sun, 2002/07/14 at 01:18:10 -0700, Luigi Rizzo wrote:
> Hi,
> the following message seems to suggest that the compiler
> (the way it is invoked) packs structures differently
> when building the kernel and userland.
>
> The stize of the structure in question is computed
> by both kernel and userland app using sizeof(),
> so there is no assumption on the size of its members,
> so i believe the only possibility of a mismatch is
> the one above.
>
> Any ideas ?
(Disclaimer: my solution below is untested, so it may all be bogus)
No, you are not accounting for "external" structure padding. Take a
look:
struct ip_fw {
struct ip_fw *next; /* linked list of rules */
u_int16_t act_ofs;/* offset of action in 32-bit units */
u_int16_t cmd_len;/* # of 32-bit words in cmd */
u_int16_t rulenum;/* rule number */
u_int16_t _pad; /* padding */
/* These fields are present in all rules. */
u_int64_t pcnt; /* Packet counter */
u_int64_t bcnt; /* Byte counter */
u_int32_t timestamp; /* tv_sec of last match */
struct ip_fw *next_rule; /* ptr to next rule */
ipfw_insn cmd[1]; /* storage for commands */
};
On a 64-bit architecture, pointers are obviously 8 bytes in size;
structure members must or should be on natural borders, depending on
the architecture.
So, next_rule will not be on a natural border; 4 bytes of padding will
be inserted before it. With that, the total structure size would be
52.
The compiler must account for the fact that an array of struct ip_fws
may be used. For obvious reasons, it can not just insert extra padding
in the array case; instead, the structure size must be chosen so that
in this situation, the first member will be on a natural border.
This results in an extra 4 bytes of "external" padding at the end,
after the member 'cmd'.
The macro you use to compute the size in the kernel is:
#define RULESIZE(rule) (sizeof(struct ip_fw) + \
((struct ip_fw *)(rule))->cmd_len * 4 - 4)
In the userland code, you start at &foo.cmd and append data
directly. This means that the padding will also be used to store
data, so the '- 4' (= sizeof(foo.cmd)) will not always be enough. The
following definition of RULESIZE (untested) should fix this:
#define RULESIZE(rule) (offsetof(struct ip_fw, cmd) + \
((struct ip_fw *)(rule))->cmd_len * 4)
It also removes the explicit 4 for sizeof(ipfw_insn).
- thomas
--
Thomas Moestl <[EMAIL PROTECTED]> http://www.tu-bs.de/~y0015675/
<[EMAIL PROTECTED]> http://people.FreeBSD.org/~tmm/
PGP fingerprint: 1C97 A604 2BD0 E492 51D0 9C0F 1FE6 4F1D 419C 776C
To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message
Re: different packing of structs in kernel vs. userland ?
Luigi Rizzo wrote:
> Hi,
> the following message seems to suggest that the compiler
> (the way it is invoked) packs structures differently
> when building the kernel and userland.
>
> The stize of the structure in question is computed
> by both kernel and userland app using sizeof(),
> so there is no assumption on the size of its members,
> so i believe the only possibility of a mismatch is
> the one above.
>
> Any ideas ?
Probably a #pragma pack() in scope.
Add one explicitly before and after the structure definition
to force it to be the same in user space and the kernel, e.g.:
#pragma pack(1) /* 1 byte packing */
struct foo {
...
};
#pragma pack() /* whatever it was before */
The latter is like a "pop" of the former's "push", so you
should be OK, even though it looks like what is happening
is an include of an include file in the wrong place or in
code that has one in scope.
-- Terry
To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message
