netdev - Re: [PATCH net] net: netlink: cap max groups which will be considered in netlink

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: <20200220.160255.1955114765293599857.davem@davemloft.net>
Date:   Thu, 20 Feb 2020 16:02:55 -0800 (PST)
From:   David Miller <davem@...emloft.net>
To:     nikolay@...ulusnetworks.com
Cc:     netdev@...r.kernel.org, christophe.leroy@....fr, rgb@...hat.com,
        erhard_f@...lbox.org
Subject: Re: [PATCH net] net: netlink: cap max groups which will be
 considered in netlink_bind()

From: Nikolay Aleksandrov <nikolay@...ulusnetworks.com>
Date: Thu, 20 Feb 2020 16:42:13 +0200

> Since nl_groups is a u32 we can't bind more groups via ->bind
> (netlink_bind) call, but netlink has supported more groups via
> setsockopt() for a long time and thus nlk->ngroups could be over 32.
> Recently I added support for per-vlan notifications and increased the
> groups to 33 for NETLINK_ROUTE which exposed an old bug in the
> netlink_bind() code causing out-of-bounds access on archs where unsigned
> long is 32 bits via test_bit() on a local variable. Fix this by capping the
> maximum groups in netlink_bind() to BITS_PER_TYPE(u32), effectively
> capping them at 32 which is the minimum of allocated groups and the
> maximum groups which can be bound via netlink_bind().
> 
> CC: Christophe Leroy <christophe.leroy@....fr>
> CC: Richard Guy Briggs <rgb@...hat.com>
> Fixes: 4f520900522f ("netlink: have netlink per-protocol bind function return an error code.")
> Reported-by: Erhard F. <erhard_f@...lbox.org>
> Signed-off-by: Nikolay Aleksandrov <nikolay@...ulusnetworks.com>

Applied.

> Dave it is not necessary to queue this fix for stable releases since
> NETLINK_ROUTE is the first to reach more groups after I added the vlan
> notification changes and I don't think we'll ever backport new groups. :)
> Up to you of course.
> 
> In fact looking at netlink_kernel_create nlk->groups can't be less than 32
> so we can add a NETLINK_MIN_GROUPS == NETLINK_MAX_LEGACY_BIND_GRPS == 32
> in net-next to replace the raw value.

Ok, thanks for letting me know. Let's skip the -stable backport.