Messages in this thread | | | Date | Fri, 12 Feb 2010 10:19:57 -0800 | From | Andrew Morton <> | Subject | Re: [PATCH] delayacct: align to 8 byte boundary on 64-bit systems |
| |
On Fri, 12 Feb 2010 11:48:27 -0500 Jeff Mahoney <jeffm@suse.com> wrote:
> prepare_reply sets up an skb for the response. If I understand it correctly, > the payload contains: > > +--------------------------------+ > | genlmsghdr - 4 bytes | > +--------------------------------+ > | NLA header - 4 bytes | /* Aggregate header */ > +-+------------------------------+ > | | NLA header - 4 bytes | /* PID header */ > | +------------------------------+ > | | pid/tgid - 4 bytes |
So we put another four zero bytes in here and add four to the "PID header".
> | +------------------------------+ > | | NLA header - 4 bytes | /* stats header */ > | + -----------------------------+ <- oops. aligned on 4 byte boundary > | | struct taskstats - 328 bytes | > +-+------------------------------+ > > The start of the taskstats struct must be 8 byte aligned on IA64 (and other > systems with 8 byte alignment rules for 64-bit types) or runtime alignment > warnings will be issued. > > This patch pads the pid/tgid field out to sizeof(long), which forces > the alignment of taskstats. The getdelays userspace code is ok with this > since it assumes 32-bit pid/tgid and then honors that header's length field. > > An array is used to avoid exposing kernel memory contents to userspace in the > response. > > Signed-off-by: Jeff Mahoney <jeffm@suse.com> > --- > kernel/taskstats.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > --- a/kernel/taskstats.c > +++ b/kernel/taskstats.c > @@ -362,6 +362,12 @@ static struct taskstats *mk_reply(struct > struct nlattr *na, *ret; > int aggr; > > + /* If we don't pad, we end up with alignment on a 4 byte boundary. > + * This causes lots of runtime warnings on systems requiring 8 byte > + * alignment */ > + u32 pids[2] = { pid, 0 }; > + int pid_size = ALIGN(sizeof(pid), sizeof(long)); > + > aggr = (type == TASKSTATS_TYPE_PID) > ? TASKSTATS_TYPE_AGGR_PID > : TASKSTATS_TYPE_AGGR_TGID; > @@ -369,7 +375,7 @@ static struct taskstats *mk_reply(struct > na = nla_nest_start(skb, aggr); > if (!na) > goto err; > - if (nla_put(skb, type, sizeof(pid), &pid) < 0) > + if (nla_put(skb, type, pid_size, pids) < 0) > goto err; > ret = nla_reserve(skb, TASKSTATS_TYPE_STATS, sizeof(struct taskstats)); > if (!ret)
So any code which assumes that the pid/tgid field is four bytes long will break. Code which takes that length from the netlink message header will work OK.
32-bit architectures are unaltered.
Seems safe enough. We'd be safer still if we didn't do this on 64-bit architectures which don't need it. ie: x86_64. But if we do that we add a risk that people will develop shoddy code which works on x86_64 and doesn't work on ia64.
hmm.
| |