linux-kernel - Re: [PATCH v1] RDMA/nldev: Add length check for IFLA_BOND_ARP_IP

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: <3c0760b5.e264b.1898a6368f8.Coremail.linma@zju.edu.cn>
Date:   Tue, 25 Jul 2023 08:11:58 +0800 (GMT+08:00)
From:   "Lin Ma" <linma@....edu.cn>
To:     "Leon Romanovsky" <leon@...nel.org>
Cc:     jgg@...pe.ca, markzhang@...dia.com, michaelgur@...dia.com,
        ohartoov@...dia.com, chenzhongjin@...wei.com, yuancan@...wei.com,
        linux-rdma@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1] RDMA/nldev: Add length check for
 IFLA_BOND_ARP_IP_TARGET parsing

Hello Leon,

> 
> On Sun, Jul 23, 2023 at 03:45:04PM +0800, Lin Ma wrote:
> > The nla_for_each_nested parsing in function
> > nldev_stat_set_counter_dynamic_doit() does not check the length of the
> > attribute. This can lead to an out-of-attribute read and allow a
> > malformed nlattr (e.g., length 0) to be viewed as a 4 byte integer.
> 
> 1. Subject of this patch doesn't really match the change.

My bad, a stupid mistake. I will fix that and prepare another patch.

> 2. See my comment on your i40e patch.
> https://lore.kernel.org/netdev/20230724174435.GA11388@unreal/
> 

Yeah I have seen that. Just as Jakub said, empty netlink attributes are valid 
(they are viewed as flag). The point is that different attribute has different
length requirement. For this specific code, the RDMA_NLDEV_ATTR_STAT_HWCOUNTERS
attribute is a nested one whose inner attributes should be NLA_U32. But as you
can see in variable nldev_policy, the description does not use nested policy to
enfore that, which results in the bug discussed in my commit message.

 [RDMA_NLDEV_ATTR_STAT_HWCOUNTERS]       = { .type = NLA_NESTED },

The elegant fix could be add the nested policy description to nldev_policy while
this is toublesome as no existing nla_attr has been given to this nested nlattr.
Hence, add the length check is the simplest solution and you can see such nla_len
check code all over the kernel.

> Thanks

Regards
Lin