netdev - Re: [PATCH] rtnetlink: fix link attribute validation with IFLA

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: <20110120160909.GC12415@cormyr>
Date:	Thu, 20 Jan 2011 18:09:10 +0200
From:	Vlad Dogaru <ddvlad@...edu.org>
To:	Patrick McHardy <kaber@...sh.net>
Cc:	NetDev <netdev@...r.kernel.org>,
	"David S. Miller" <davem@...emloft.net>
Subject: Re: [PATCH] rtnetlink: fix link attribute validation with
 IFLA_GROUP

On Thu, Jan 20, 2011 at 02:00:42PM +0100, Patrick McHardy wrote:
> Fix a few semantic problems with the new IFLA_GROUP attribute.
> 
> Vlad, could you please give this is a try to see whether it
> still behaves as expected?

> commit e4b31d565a45e06ed2e51a005f5c00ff1d00725c
> Author: Patrick McHardy <kaber@...sh.net>
> Date:   Thu Jan 20 13:55:25 2011 +0100
> 
>     rtnetlink: fix link attribute validation with IFLA_GROUP
>     
>     rtnl_group_changelink() is invoked by rtnl_newlink() before the link
>     attributes have been validated. Additionally the group changes are
>     performed even if NLM_F_CREATE is specified and a new link is
>     created, while more reasonable semantics would be to set the group
>     value on the newly created link.
>     
>     Fix both problems by moving the rtnl_group_changelink() invocation
>     down to the handling of non-existant links without NLM_F_CREATE()
>     and add a dev_set_group() call to rtnl_create_link().
>     
>     Signed-off-by: Patrick McHardy <kaber@...sh.net>

Acked-by: Vlad Dogaru <ddvlad@...edu.org>

This looks OK and behaves as before. Thanks for taking the time to look
through it, the new semantics do seem saner.

There is a slight difference, though: in the (yet unapplied) iproute2
patch, ifi_index is set to -1 and kernel space checks that it is <= 0;
your patch checks for == 0. I will update the iproute2 patches with
setting ifi_index to 0 (and other changes) and resubmit them.
--
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