[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <OF326F3AF9.7322F5FB-ON88257437.00174874-88257437.00194F7B@us.ibm.com>
Date: Fri, 25 Apr 2008 21:36:29 -0700
From: David Stevens <dlstevens@...ibm.com>
To: YOSHIFUJI Hideaki / 吉藤英明
<yoshfuji@...ux-ipv6.org>
Cc: davem@...emloft.net, netdev@...r.kernel.org,
yoshfuji@...ux-ipv6.org
Subject: Re: [GIT PULL] [IPV6] COMPAT: Fix SSM applications on 64bit kernels.
YOSHIFUJI Hideaki / 吉藤英明 <yoshfuji@...ux-ipv6.org> wrote on 04/25/2008
08:33:59 PM:
> Please define __compat_sockaddr_storage{} with attribute((aligned(4))).
> If you do that we do not need attribute(packed). No?
I just tried it and the compiler seems to do that for this case,
but the gcc documentation says explicitly that you need both packed and
aligned if you want it to be a specific value. aligned(4) is a minimum
only and the compiler would be free to add pad still as long as the
padded value was also 4-byte aligned.
In this case, we want no pad, so packed is the relevant part. If
had a char followed by a struct as in your previous example, the right
way to get 4-byte alignment is both aligned(4) and packed. Leaving off
packed would allow 8-byte or 16-byte (or 12 :-)) alignment, too.
>
> > +extern int compat_mc_setsockopt(struct sock *, int, int, char __user
*,
> > int);
> > +
>
> compat_do_inet_mc_setsockopt() or so.
>
> > #endif /* NET_COMPAT_H */
> > diff -ruNp linux-2.6.18.ppc64/net/compat.c
> > linux-2.6.18.ppc64DLS1/net/compat.c
> > --- linux-2.6.18.ppc64/net/compat.c 2008-04-21 13:51:02.000000000
> > -0700
> > +++ linux-2.6.18.ppc64DLS1/net/compat.c 2008-04-25 08:53:47.000000000
> > -0700
>
> net/compat.c is not the right place.
> Please put this in net/ipv4/compat.c or so.
> Or at least, these must be guarded by CONFIG_INET.
This code is for both v4 and v6, so I'd rather it not be in
net/ipv4/compat.c. I didn't try without CONFIG_INET, -- will look
at that.
> > +struct compat_group_filter {
> > + __u32 gf_interface;
> > + struct __kernel_sockaddr_storage gf_group
> > + __attribute__ ((aligned(4)));
> > + __u32 gf_fmode;
> > + __u32 gf_numsrc;
> > + struct __kernel_sockaddr_storage gf_slist[1]
> > + __attribute__ ((aligned(4)));
> > +} __attribute__ ((packed));
> > +
>
> include/linux/in.n.
I don't understand this comment. If you're suggesting I
put these in an include file, I didn't because this is the only
file that uses them and I don't want them to be visible to any
other code. If you mean something else, please explain.
>
> __copy_in_user().
> Maybe we can use __copy_in_user for u32?
I'll look at it.
thanks!
+-DLS
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists