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: <20220623090352.69bf416c@kernel.org>
Date:   Thu, 23 Jun 2022 09:03:52 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     David Ahern <dsahern@...il.com>
Cc:     Ismael Luceno <iluceno@...e.de>,
        "David S. Miller" <davem@...emloft.net>,
        Paolo Abeni <pabeni@...hat.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: Netlink NLM_F_DUMP_INTR flag lost

On Wed, 22 Jun 2022 22:01:37 -0600 David Ahern wrote:
> > Right, the question is what message can we introduce here which would
> > not break old user space?  
> 
> I would hope a "normal"

"normal" == with no attributes?

> message with just the flags set is processed by
> userspace. iproute2 does - lib/libnetlink.c, rtnl_dump_filter_l(). It
> checks the nlmsg_flags first.

🤞

> > The alternative of not wiping the _DUMP_INTR flag as we move thru
> > protocols seems more and more appealing, even tho I was initially
> > dismissive.
> > 
> > We should make sure we do one last consistency check before we return 0
> > from the handlers. Or even at the end of the loop in rtnl_dump_all().  
> 
> Seems like netlink_dump_done should handle that for the last dump?

Yeah, the problem is:
 - it gets lost between families when dumping all, and
 - if the dump get truncated _DUMP_INTR never gets set because many
   places only check consistency when outputting an object.

> That said, in rtnl_dump_all how about a flags check after dumpit() and
> send the message if INTR is set? would need to adjust the return code of
> rtnl_dump_all so netlink_dump knows the dump is not done yet.

Yup, the question for me is what's the risk / benefit of sending 
the empty message vs putting the _DUMP_INTR on the next family.
I'm leaning towards putting it on the next family and treating 
the entire dump as interrupted, do you reckon that's suboptimal?
User space can always dump family by family if it cares about
breaking the entire dump.

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index ac45328607f7..c36874d192ef 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -3879,6 +3879,7 @@ static int rtnl_dump_all(struct sk_buff *skb, struct netlink_callback *cb)
 			continue;
 
 		if (idx > s_idx) {
+			nl_dump_check_consistent(cb, nlmsg_hdr(skb));
 			memset(&cb->args[0], 0, sizeof(cb->args));
 			cb->prev_seq = 0;
 			cb->seq = 0;