netdev - Re: [PATCH] rtnetlink: fix data overflow in rtnl

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20201019101555.64b9f723@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Mon, 19 Oct 2020 10:15:55 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     "zhudi (J)" <zhudi21@...wei.com>
Cc:     Michal Kubecek <mkubecek@...e.cz>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "Chenxiang (EulerOS)" <rose.chen@...wei.com>
Subject: Re: [PATCH] rtnetlink: fix data overflow in rtnl_calcit()

On Mon, 19 Oct 2020 01:59:19 +0000 zhudi (J) wrote:
> > zhudi, why not use size_t? Seems like the most natural fit for counting size.  
> 
> Thanks for your replying.
> min_dump_alloc original type used is u16 and it's eventually assigned to 
> struct netlink_callback{}. min_dump_alloc which data type is u32. So I just simply
> promote to u32.
> Should be used size_t instead of u32?

I had a closer look, and I agree that u32 should be fine in struct
netlink_dump_control, rtnetlink_rcv_msg(), and as a return value from
rtnl_calcit().

But please use size_t for the local variable in rtnl_calcit().
This way you can convert the max_t() to a max().

When you send v2 please move the declaration of min_ifinfo_dump_size
after struct net *net = sock_net(skb->sk); (to get closer to longest 
to shortest declaration order).