netdev - RE: [PATCH 2/3] netlink: nla_parse_nested_compat was not parsing nested attributes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <80769D7B14936844A23C0C43D9FBCF0F14E9B94A@orsmsx501.amr.corp.intel.com>
Date:	Fri, 22 Aug 2008 10:40:07 -0700
From:	"Duyck, Alexander H" <alexander.h.duyck@...el.com>
To:	David Miller <davem@...emloft.net>,
	"Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>
CC:	"jeff@...zik.org" <jeff@...zik.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"tgraf@...g.ch" <tgraf@...g.ch>,
	"shemminger@...l.org" <shemminger@...l.org>,
	"kaber@...sh.net" <kaber@...sh.net>
Subject: RE: [PATCH 2/3] netlink: nla_parse_nested_compat was not parsing
 nested attributes

I figured I would CC Stephen Hemminger and Patrick McHardy since they also will likely have an interest in this.

Just to put together a quick history on this issue I will list off how things got to where they are:

1.  "[NET_SCHED]: sch_netem: use nla_parse_nested_compat" (http://marc.info/?l=linux-netdev&m=120110639320780&w=2) from Patrick McHardy.  After this patch netem started reporting "bytes leftover after parsing attributes" errors as it wasn't actually receiving a nested compat attribute.  This patch was added around 2.6.25-rc1.

2.  "[NETLINK]: Fix nla_parse_nested_compat() to call nla_parse() directly" (http://marc.info/?l=linux-netdev&m=121145625328370&w=2) from Thomas Graf.  This resolved the errors but in turn any netlink messages that generated correct nested netlink attributes would have those attributes ignored as they would be automatically parsed out as one attribute with a length of all the nested attributes.  This patch was added around 2.6.26-rc4.

Patches 3 & 4 haven't been applied yet and are under review:

3.  "[PATCH 2/3] netlink: nla_parse_nested_compat was not parsing nested attributes" (http://marc.info/?l=linux-netdev&m=121936623112195&w=2) from me.  This reverts patch 2.

4.  "[PATCH] IPROUTE: correct nla nested message generated by netem_parse_opt" (http://marc.info/?l=linux-netdev&m=121936623112195&w=2) from me.  This patch changes iproute2 netem_parse_opt to generate a correctly formatted set of nested compat attributes that can be parsed after the introduction of patch 1.

The way I see it we have 3 possible solutions to all of this.  First there is my proposed solution in patches 3 & 4.  The second option would be to revert patch 1 and apply patch 3 which would maintain the kernel ABI and not require any patching to iproute2.  The third option I see would be to go back and patch include/net/netlink.h in the kernel and lib/libnetlink.c in iproute2 so that they all support the new format which is parsed by nla_parse_nested_compat.

Thanks,

Alex

-----Original Message-----
From: David Miller [mailto:davem@...emloft.net]
Sent: Friday, August 22, 2008 3:18 AM
To: Kirsher, Jeffrey T
Cc: jeff@...zik.org; netdev@...r.kernel.org; Duyck, Alexander H; tgraf@...g.ch
Subject: Re: [PATCH 2/3] netlink: nal_parse_nested_compat was not parsing nested attributes

From: Jeff Kirsher <jeffrey.t.kirsher@...el.com>
Date: Thu, 21 Aug 2008 17:51:26 -0700

Please at least CC: the author of the patch you are reverting so
that they can provide feedback and have a look at this.

Thomas Graf CC:'d.

> From: Alexander Duyck <alexander.h.duyck@...el.com>
>
> This patch reverts previous commit: b9a2f2e450b0f770bb4347ae8d48eb2dea701e24
> netlink: Fix nla_parse_nested_compat() to call nla_parse() directly
>
> The purpose of nla_parse_nested_compat is to parse attributes which contain a
> struct followed by a container attribute with a stream of nested attributes.
> This patch reverts the previous patch which assumed that there was no container
> attribute due to a malformed string of attributes being generated by
> netem_parse_opts
>
> Signed-off-by: Alexander Duyck <alexander.h.duyck@...el.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@...el.com>
> ---
>
>  include/net/netlink.h |   11 +++++------
>  1 files changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/include/net/netlink.h b/include/net/netlink.h
> index 18024b8..b295cd1 100644
> --- a/include/net/netlink.h
> +++ b/include/net/netlink.h
> @@ -770,13 +770,12 @@ static inline int __nla_parse_nested_compat(struct nlattr *tb[], int maxtype,
>                                           const struct nla_policy *policy,
>                                           int len)
>  {
> -     int nested_len = nla_len(nla) - NLA_ALIGN(len);
> -
> -     if (nested_len < 0)
> +     if (nla_len(nla) < len)
>               return -EINVAL;
> -     if (nested_len >= nla_attr_size(0))
> -             return nla_parse(tb, maxtype, nla_data(nla) + NLA_ALIGN(len),
> -                              nested_len, policy);
> +     if (nla_len(nla) >= NLA_ALIGN(len) + sizeof(struct nlattr))
> +             return nla_parse_nested(tb, maxtype,
> +                                     nla_data(nla) + NLA_ALIGN(len),
> +                                     policy);
>       memset(tb, 0, sizeof(struct nlattr *) * (maxtype + 1));
>       return 0;
>  }
>
--
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