netdev - Re: Netlink NLM_F_DUMP

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: <20220824125901.21a28927@pirotess>
Date:   Wed, 24 Aug 2022 12:59:01 +0200
From:   Ismael Luceno <iluceno@...e.de>
To:     David Ahern <dsahern@...il.com>
Cc:     Jakub Kicinski <kuba@...nel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Paolo Abeni <pabeni@...hat.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Florian Westphal <fw@...len.de>
Subject: Re: Netlink NLM_F_DUMP_INTR flag lost

On 17/Jun/2022 10:28, David Ahern wrote:
> On 6/17/22 9:22 AM, Jakub Kicinski wrote:
<...>
> > FWIW what I think is strange is that we record the gen id before the
> > dump and then check if the recorded version was old. Like.. what's
> > the point of that? Nothing updates cb->seq while dumping AFAICS, so
> > the code is functionally equivalent to this right?
<...> 
> FWIW, net/ipv4/nexthop.c sets cb->seq at the end of the loop and the
> nl_dump_check_consistent follows that.

[CCing Florian Westphal because he gave a related talk at netdev 2.2,
 maybe he can add something]

It seems to me now that most of the calls to nl_dump_check_consistent
are redundant.

Either the rtnl lock is explicitly taken:
- ethnl_tunnel_info_dumpit

Or are implicitly called with the rtnl lock held:
- rtnl_dump_ifinfo
- rtnl_dump_all
- in_dev_dump_addr
- inet_netconf_dump_devconf
- rtm_dump_nexthop
- rtm_dump_nexthop_bucket
- mpls_netconf_dump_devconf

Except:
- inet6_netconf_dump_devconf
- inet6_dump_addr

I assume the ones that rely on rcu_read_lock are safe too.

Also, the following ones set cb->seq just before calling it:
- rtm_dump_nh_ctx
- rtm_dump_res_bucket_ctx

Does it make sense to remove these calls or is anyone looking forward
to convert the functions to run without the rtnl lock?

Am I missing something here?

-- 
Ismael Luceno
SUSE - Support Level 3