netdev - Re: [PATCH net-next] rtnetlink: add RTNH_REJECT

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] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 30 Nov 2021 12:28:17 +0200
From:   Ido Schimmel <idosch@...sch.org>
To:     Alexander Mikhalitsyn <alexander.mikhalitsyn@...tuozzo.com>,
        roopa@...dia.com
Cc:     David Ahern <dsahern@...il.com>, netdev@...r.kernel.org,
        David Miller <davem@...emloft.net>,
        Stephen Hemminger <stephen@...workplumber.org>,
        Ido Schimmel <idosch@...dia.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Andrei Vagin <avagin@...il.com>,
        Pavel Tikhomirov <ptikhomirov@...tuozzo.com>,
        Alexander Mikhalitsyn <alexander@...alicyn.com>
Subject: Re: [PATCH net-next] rtnetlink: add RTNH_REJECT_MASK

On Tue, Nov 30, 2021 at 12:53:52PM +0300, Alexander Mikhalitsyn wrote:
> On Tue, 30 Nov 2021 11:28:32 +0200
> Ido Schimmel <idosch@...sch.org> wrote:
> 
> > On Tue, Nov 30, 2021 at 11:35:17AM +0300, Alexander Mikhalitsyn wrote:
> > > On Tue, 30 Nov 2021 09:59:25 +0200
> > > Ido Schimmel <idosch@...sch.org> wrote:
> > > > Looking at the patch again, what is the motivation to expose
> > > > RTNH_REJECT_MASK to user space? iproute2 already knows that it only
> > > > makes sense to set RTNH_F_ONLINK. Can't we just do:
> > > 
> > > Sorry, but that's not fully clear for me, why we should exclude RTNH_F_ONLINK?
> > > I thought that we should exclude RTNH_F_DEAD and RTNH_F_LINKDOWN just because
> > > kernel doesn't allow to set these flags.
> > 
> > I don't think we should exclude RTNH_F_ONLINK. I'm saying that it is the
> > only flag that it makes sense to send to the kernel in the ancillary
> > header of RTM_NEWROUTE messages. The rest of the RNTH_F_* flags are
> > either not used by the kernel or are only meant to be sent from the
> > kernel to user space. Due to omission, they are mistakenly allowed.
> 
> Ah, okay, so, the patch should be like
> 
> diff --git a/ip/iproute.c b/ip/iproute.c
> index 1447a5f78f49..0e6dad2b67e5 100644
> --- a/ip/iproute.c
> +++ b/ip/iproute.c
> @@ -1632,6 +1632,8 @@ static int save_route(struct nlmsghdr *n, void *arg)
>         if (!filter_nlmsg(n, tb, host_len))
>                 return 0;
>  
> +       r->rtm_flags &= RTNH_F_ONLINK;
> +
>         ret = write(STDOUT_FILENO, n, n->nlmsg_len);
>         if ((ret > 0) && (ret != n->nlmsg_len)) {
>                 fprintf(stderr, "Short write while saving nlmsg\n");
> 
> to filter out all flags *except* RTNH_F_ONLINK.

Yes

> 
> But what about discussion from
> https://lore.kernel.org/netdev/ff405eae-21d9-35f4-1397-b6f9a29a57ff@nvidia.com/
> 
> As far as I understand Roopa, we have to save at least RTNH_F_OFFLOAD flag too,
> for instance, if user uses Cumulus and want to dump/restore routes.
> 
> I'm sorry if I misunderstood something.

Roopa, do you see a problem with the above patch?