Subject: Re: [Fwd: Re: connector is missing in 2.6.12-rc2-mm1]

From: Evgeniy Polyakov <[email-masked]>
Date: Mon, 11 Apr 2005 15:19:53 +0400

On Mon, 2005-04-11 at 12:45 +0200, Thomas Graf wrote:
> * Evgeniy Polyakov <[email-masked]> 2005-04-11 09:22
> > On Sun, Apr 10, 2005 at 09:27:27PM +0200, Thomas Graf ([email-masked]) wrote:
> > > +       size = NLMSG_SPACE(sizeof(*msg) + msg->len);
> > > +
> > > +       skb = alloc_skb(size, GFP_ATOMIC);
> > > +       if (!skb) {
> > > +               printk(KERN_ERR "Failed to allocate new skb with size=%u.\n", size);
> > > +               return;
> > > +       }
> > > +
> > > +       nlh = NLMSG_PUT(skb, 0, msg->seq, NLMSG_DONE, size - sizeof(*nlh));
> > > 
> > > This is not correct, what happens is:
> > > size = NLMSG_SPACE(sizeof(*msg) + msg->len);
> > >  --> align(hdr)+align(data)
> > > size - sizeof(*nlh)
> > >  --> (align(hdr)-hdr)+align(data)
> > > NLMSG_PUT pads again to get to the end of the data block (NLMSG_LENGTH)
> > >  --> align(hdr)+(align(hdr)-hdr)+align(data)
> > > 
> > > At the moment align(hdr) == hdr since nlmsghdr is already aligned
> > > but this might change and your code will break.
> > 
> > As far as I remember, header is always supposed to be aligned properly
> > "by design", so it even could be nonaligned here.
> 
> No, have a look at the macros:
> 
> #define NLMSG_LENGTH(len) ((len)+NLMSG_ALIGN(sizeof(struct nlmsghdr)))
> #define NLMSG_SPACE(len) NLMSG_ALIGN(NLMSG_LENGTH(len))
> 
> NLMSG_LENGTH points to the end of the payload in the message, NLMSG_SPACE
> represents the total size aligned properly for a possible next multipart
> message.
> 
> It is unlikely that nlmsghdr will ever be unaligned but there can be no
> reason to introduce code that can break with perfectly legal changes just
> because of that.

But NLMSG_ALIGN(sizeof(hdr)) is always equal to sizeof(hdr), that size
was select not accidentally.

I will change it, no problem, it is good from aesthetic point of view.

-- 
        Evgeniy Polyakov

Crash is better than data corruption -- Arthur Grabowski