netdev - Re: [PATCH v1 1/2] xfrm: Check if_id in xfrm

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: <CADHa2dAaG4Pgxk7gmDbBnVSYJ_eBtJY3KaR94fY=wp+Pmt0EoA@mail.gmail.com>
Date:   Wed, 12 Jan 2022 14:53:31 -0800
From:   Yan Yan <evitayan@...gle.com>
To:     Steffen Klassert <steffen.klassert@...unet.com>
Cc:     netdev@...r.kernel.org, Herbert Xu <herbert@...dor.apana.org.au>,
        "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Lorenzo Colitti <lorenzo@...gle.com>,
        Maciej Żenczykowski <maze@...gle.com>,
        Nathan Harold <nharold@...gle.com>,
        Benedict Wong <benedictwong@...gle.com>
Subject: Re: [PATCH v1 1/2] xfrm: Check if_id in xfrm_migrate

Hi Steffen,

The Jan 7th patch fixes the following warning (reported by the kernel
test robot) by adding parentheses.
   net/xfrm/xfrm_policy.c: In function 'xfrm_migrate':
>> net/xfrm/xfrm_policy.c:4403:21: warning: suggest parentheses around assignment used as truth value [-Wparentheses]
    4403 |                 if (x = xfrm_migrate_state_find(mp, net, if_id)) {
         |                     ^

In the Jan 7th patch, this line becomes "if ((x =
xfrm_migrate_state_find(mp, net, if_id))) {"


On Tue, Jan 11, 2022 at 11:32 PM Steffen Klassert
<steffen.klassert@...unet.com> wrote:
>
> On Fri, Jan 07, 2022 at 05:32:30PM -0800, Yan Yan wrote:
> > This patch enables distinguishing SAs and SPs based on if_id during
> > the xfrm_migrate flow. This ensures support for xfrm interfaces
> > throughout the SA/SP lifecycle.
> >
> > When there are multiple existing SPs with the same direction,
> > the same xfrm_selector and different endpoint addresses,
> > xfrm_migrate might fail with ENODATA.
> >
> > Specifically, the code path for performing xfrm_migrate is:
> >   Stage 1: find policy to migrate with
> >     xfrm_migrate_policy_find(sel, dir, type, net)
> >   Stage 2: find and update state(s) with
> >     xfrm_migrate_state_find(mp, net)
> >   Stage 3: update endpoint address(es) of template(s) with
> >     xfrm_policy_migrate(pol, m, num_migrate)
> >
> > Currently "Stage 1" always returns the first xfrm_policy that
> > matches, and "Stage 3" looks for the xfrm_tmpl that matches the
> > old endpoint address. Thus if there are multiple xfrm_policy
> > with same selector, direction, type and net, "Stage 1" might
> > rertun a wrong xfrm_policy and "Stage 3" will fail with ENODATA
> > because it cannot find a xfrm_tmpl with the matching endpoint
> > address.
> >
> > The fix is to allow userspace to pass an if_id and add if_id
> > to the matching rule in Stage 1 and Stage 2 since if_id is a
> > unique ID for xfrm_policy and xfrm_state. For compatibility,
> > if_id will only be checked if the attribute is set.
> >
> > Tested with additions to Android's kernel unit test suite:
> > https://android-review.googlesource.com/c/kernel/tests/+/1668886
> >
> > Signed-off-by: Yan Yan <evitayan@...gle.com>
>
> What is the difference between this patch and the one with
> the same subject you sent on Jan 5th?



-- 
--
Best,
Yan