netdev - Re: [PATCH net-next] xfrm: Simplify SA looking up when using wildcard source address

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: <20130924114551.GT7660@secunet.com>
Date:	Tue, 24 Sep 2013 13:45:51 +0200
From:	Steffen Klassert <steffen.klassert@...unet.com>
To:	Fan Du <fan.du@...driver.com>
Cc:	davem@...emloft.net, netdev@...r.kernel.org
Subject: Re: [PATCH net-next] xfrm: Simplify SA looking up when using
 wildcard source address

On Mon, Sep 23, 2013 at 05:18:37PM +0800, Fan Du wrote:
> I'm not quite sure I get this "wildcard source address" right,
> IMHO if a host needs to protect every traffic for a given remote host,
> then the source address is wildcard address, i.e. all ZEROs.
> (Please correct me if I'm bloodly wrong。。。)

The above does not belong to a commit message, really.
If you are not sure and you want comments on your patch,
mark your patch as RFC. You should be sure that your patch
is correct when you submit, at least in the moment you
send it. I know that this can change a second after,
but in that moment you should be sure.

> 
> Here is the argument if above statement stands true:
> __xfrm4/6_state_addr_check is a four steps check, all we need to do
> is checking whether the destination address match. Passing saddr from
> flow is worst option, as the checking needs to reach the fourth step.
> 
> So, simply this process by only checking destination address only when
> using wildcard source address for looking up SAs.
> 
> Signed-off-by: Fan Du <fan.du@...driver.com>
> ---

If you have further comments on your patch that should not be
included in the commit message, you can add them here.

>  include/net/xfrm.h    |   31 +++++++++++++++++++++++++++++++
>  net/xfrm/xfrm_state.c |    2 +-
>  2 files changed, 32 insertions(+), 1 deletion(-)
> 
> diff --git a/include/net/xfrm.h b/include/net/xfrm.h
> index e253bf0..fdb9343 100644
> --- a/include/net/xfrm.h
> +++ b/include/net/xfrm.h
> @@ -1282,6 +1282,37 @@ xfrm_state_addr_check(const struct xfrm_state *x,
>  }
>  
>  static __inline__ int
> +__xfrm4_state_daddr_check(const struct xfrm_state *x,
> +                                const xfrm_address_t *daddr)
> +{
> +        return ((daddr->a4 == x->id.daddr.a4) ? 1 : 0);
> +}
> +
> +static __inline__ int
> +__xfrm6_state_daddr_check(const struct xfrm_state *x,
> +                         const xfrm_address_t *daddr)
> +{
> +        if (ipv6_addr_equal((struct in6_addr *)daddr, (struct in6_addr *)&x->id.daddr))
> +                return 1;
> +        else 
> +                return 0;
> +}
> +
> +static __inline__ int
> +xfrm_state_daddr_check(const struct xfrm_state *x,
> +                      const xfrm_address_t *daddr,
> +                      unsigned short family)
> +{
> +        switch (family) {
> +        case AF_INET:
> +                return __xfrm4_state_daddr_check(x, daddr);
> +        case AF_INET6:
> +                return __xfrm6_state_daddr_check(x, daddr);
> +        }    
> +        return 0;
> +}

You used whitespaces where you should use tabs in the whole patch.
Please do the formating right to avoid cleanup patches.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html