netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Thomas Graf <tgraf@suug.ch>
To: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Cc: bhutchings@solarflare.com, netdev@vger.kernel.org,
	davem@davemloft.net, David.Laight@ACULAB.COM
Subject: Re: [PATCH v2] netlink: align attributes on 64-bits
Date: Wed, 19 Dec 2012 17:09:12 +0000	[thread overview]
Message-ID: <20121219170912.GB6975@casper.infradead.org> (raw)
In-Reply-To: <50D1A37C.8090705@6wind.com>

On 12/19/12 at 12:22pm, Nicolas Dichtel wrote:
> Here padlen will return 4, which is wrong: padlen + NLA_HDRLEN = 8,
> alignment is the same than before. Here is a proposal fix:
> 
> diff --git a/lib/nlattr.c b/lib/nlattr.c
> index e4f0329..1556313 100644
> --- a/lib/nlattr.c
> +++ b/lib/nlattr.c
> @@ -338,7 +338,10 @@ struct nlattr *__nla_reserve(struct sk_buff
> *skb, int attrtype, int attrlen)
>  		struct nlattr *pad;
>  		size_t padlen;
> 
> -		padlen = nla_total_size(offset) - offset -  NLA_HDRLEN;
> +		/* We need to remove NLA_HDRLEN two times: one time for the
> +		 * attribute hdr and one time for the pad attribute hdr.
> +		 */
> +		padlen = nla_total_size(offset) - offset -  2 * NLA_HDRLEN;
>  		pad = (struct nlattr *) skb_put(skb, nla_attr_size(padlen));
>  		pad->nla_type = 0;
>  		pad->nla_len = nla_attr_size(padlen);
> 
> With this patch, it seems goods. attribute are always aligned on 8 bytes. Also
> I did not notice any problem with size calculation (I try some ip
> link, ip xfrm, ip [m]route).
> 
> Do you want to make more tests? Or will your repost the full patch?
> I can do it if you don't have time.

Thanks.

I would like to do some testing as well. I do expect some fallout from
this. There is likely some interface abuse that will now be exposed
due to this.

We'll have to wait for the next merge window to open anyway. I'd
consider this a new feature and not a bugfix based on the possible
regression impact it could have.

I'll post a new version of the patch integrating your fix above so
others (especially subsystem maintainers depending on netlink) can run
the patch as well.

  reply	other threads:[~2012-12-19 17:09 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-12-04 11:13 [PATCH net-next 0/7] Allow to monitor multicast cache event via rtnetlink Nicolas Dichtel
2012-12-04 11:13 ` [PATCH net-next 1/7] netconf: advertise mc_forwarding status Nicolas Dichtel
2012-12-04 11:13 ` [PATCH net-next 2/7] ip6mr: use nla_nest_* helpers Nicolas Dichtel
2012-12-04 11:13 ` [PATCH net-next 3/7] ipmr/ip6mr: advertise mfc stats via rtnetlink Nicolas Dichtel
2012-12-04 11:13 ` [PATCH net-next 4/7] ipmr/ip6mr: report origin of mfc entry into rtnl msg Nicolas Dichtel
2012-12-04 11:13 ` [PATCH net-next 5/7] ipmr/ip6mr: allow to get unresolved cache via netlink Nicolas Dichtel
2012-12-04 11:13 ` [PATCH net-next 6/7] ipmr: advertise new mfc entries via rtnl Nicolas Dichtel
2012-12-04 11:13 ` [PATCH net-next 7/7] ip6mr: " Nicolas Dichtel
2012-12-04 18:09 ` [PATCH net-next 0/7] Allow to monitor multicast cache event via rtnetlink David Miller
2012-12-04 20:02   ` Nicolas Dichtel
2012-12-05 11:02     ` Nicolas Dichtel
2012-12-05 11:41       ` David Laight
2012-12-05 17:54         ` David Miller
2012-12-06  8:43           ` Nicolas Dichtel
2012-12-06 17:49             ` Thomas Graf
2012-12-06 21:49               ` Nicolas Dichtel
2012-12-07 10:38               ` David Laight
2012-12-07 10:58                 ` Thomas Graf
2012-12-11 15:03               ` Nicolas Dichtel
2012-12-11 18:40                 ` Thomas Graf
2012-12-12 17:30                   ` Nicolas Dichtel
2012-12-14 13:16                   ` [PATCH] netlink: align attributes on 64-bits Nicolas Dichtel
2012-12-14 15:49                     ` Ben Hutchings
2012-12-14 16:04                       ` Nicolas Dichtel
2012-12-17 16:49                       ` [PATCH v2] " Nicolas Dichtel
2012-12-17 17:06                         ` David Laight
2012-12-17 17:35                           ` Nicolas Dichtel
2012-12-18  9:19                             ` David Laight
2012-12-18 10:18                               ` Nicolas Dichtel
2012-12-18 12:57                         ` Thomas Graf
2012-12-18 16:23                           ` Nicolas Dichtel
2012-12-18 16:50                             ` David Laight
2012-12-18 17:11                               ` Thomas Graf
2012-12-19  9:17                                 ` David Laight
2012-12-19 17:20                                   ` Thomas Graf
2012-12-20  9:37                                     ` David Laight
2012-12-20  9:40                                     ` David Laight
2012-12-18 17:08                             ` Thomas Graf
2012-12-18 22:07                               ` Nicolas Dichtel
2012-12-19 11:22                           ` Nicolas Dichtel
2012-12-19 17:09                             ` Thomas Graf [this message]
2012-12-19 18:07                               ` Nicolas Dichtel
2012-12-17  9:59                     ` [PATCH] " David Laight
2012-12-17 16:53                       ` Nicolas Dichtel
2012-12-05 17:53       ` [PATCH net-next 0/7] Allow to monitor multicast cache event via rtnetlink David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20121219170912.GB6975@casper.infradead.org \
    --to=tgraf@suug.ch \
    --cc=David.Laight@ACULAB.COM \
    --cc=bhutchings@solarflare.com \
    --cc=davem@davemloft.net \
    --cc=netdev@vger.kernel.org \
    --cc=nicolas.dichtel@6wind.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).