netdev - Re: [PATCH net-next v2 1/7] net: ip: make fib_validate

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: <CADxym3baw2nLvANd-D5D2kCNRRoDmdgexBeGmD-uCcYYqAf=EQ@mail.gmail.com>
Date: Thu, 10 Oct 2024 17:18:45 +0800
From: Menglong Dong <menglong8.dong@...il.com>
To: Paolo Abeni <pabeni@...hat.com>
Cc: edumazet@...gle.com, kuba@...nel.org, davem@...emloft.net, 
	dsahern@...nel.org, steffen.klassert@...unet.com, herbert@...dor.apana.org.au, 
	dongml2@...natelecom.cn, bigeasy@...utronix.de, toke@...hat.com, 
	idosch@...dia.com, netdev@...r.kernel.org, linux-kernel@...r.kernel.org, 
	bpf@...r.kernel.org
Subject: Re: [PATCH net-next v2 1/7] net: ip: make fib_validate_source()
 return drop reason

On Thu, Oct 10, 2024 at 4:25 PM Paolo Abeni <pabeni@...hat.com> wrote:
>
>
>
> On 10/7/24 09:46, Menglong Dong wrote:
> > In this commit, we make fib_validate_source/__fib_validate_source return
> > -reason instead of errno on error. As the return value of them can be
> > -errno, 0, and 1, we can't make it return enum skb_drop_reason directly.
> >
> > In the origin logic, if __fib_validate_source() return -EXDEV,
> > LINUX_MIB_IPRPFILTER will be counted. And now, we need to adjust it by
> > checking "reason == SKB_DROP_REASON_IP_RPFILTER". However, this will take
> > effect only after the patch "net: ip: make ip_route_input_noref() return
> > drop reasons", as we can't pass the drop reasons from
> > fib_validate_source() to ip_rcv_finish_core() in this patch.
> >
> > We set the errno to -EINVAL when fib_validate_source() is called and the
> > validation fails, as the errno can be checked in the caller and now its
> > value is -reason, which can lead misunderstand.
> >
> > Following new drop reasons are added in this patch:
> >
> >    SKB_DROP_REASON_IP_LOCAL_SOURCE
> >    SKB_DROP_REASON_IP_INVALID_SOURCE
> >
> > Signed-off-by: Menglong Dong <dongml2@...natelecom.cn>
>
> Looking at the next patches, I'm under the impression that the overall
> code will be simpler if you let __fib_validate_source() return directly
> a drop reason, and fib_validate_source(), too. Hard to be sure without
> actually do the attempt... did you try such patch by any chance?
>

I analysed the usages of fib_validate_source() before. The
return value of fib_validate_source() can be -errno, "0", and "1".
And the value "1" can be used by the caller, such as
__mkroute_input(). Making it return drop reasons can't cover this
case.

It seems that __mkroute_input() is the only case that uses the
positive returning value of fib_validate_source(). Let me think
about it more in this case.

Thanks!
Menglong Dong

> Thanks!
>
> Paolo
>