Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 31 May 2022 11:06:57 +0200
From:      Dimitry Andric <dim@FreeBSD.org>
To:        Kristof Provost <kp@freebsd.org>
Cc:        Gleb Smirnoff <glebius@FreeBSD.org>, "src-committers@freebsd.org" <src-committers@FreeBSD.org>, "dev-commits-src-all@freebsd.org" <dev-commits-src-all@FreeBSD.org>, "dev-commits-src-branches@freebsd.org" <dev-commits-src-branches@FreeBSD.org>
Subject:   Re: git: 4dfd3ffc4488 - stable/13 - if: avoid interface destroy race
Message-ID:  <1F175CE2-7649-4899-9CA0-8ADCE9206B6A@FreeBSD.org>
In-Reply-To: <202205271637.24RGbHHW010235@gitrepo.freebsd.org>
References:  <202205271637.24RGbHHW010235@gitrepo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help

--Apple-Mail=_F9F1D825-98E6-482B-AFE0-F0F9BBB7E2D1
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
	charset=us-ascii

On 27 May 2022, at 18:37, Kristof Provost <kp@freebsd.org> wrote:
>=20
> The branch stable/13 has been updated by kp:
>=20
> URL: =
https://cgit.FreeBSD.org/src/commit/?id=3D4dfd3ffc4488e5e2662cdc40deec17d8=
2432da0b
>=20
> commit 4dfd3ffc4488e5e2662cdc40deec17d82432da0b
> Author:     Kristof Provost <kp@FreeBSD.org>
> AuthorDate: 2022-03-27 18:23:25 +0000
> Commit:     Kristof Provost <kp@FreeBSD.org>
> CommitDate: 2022-05-27 16:25:10 +0000
>=20
>    if: avoid interface destroy race
>=20
>    When we destroy an interface while the jail containing it is being
>    destroyed we risk seeing a race between if_vmove() and the =
destruction
>    code, which results in us trying to move a destroyed interface.
>=20
>    Protect against this by using the ifnet_detach_sxlock to also =
covert
>    if_vmove() (and not just detach).
>=20
>    PR:             262829
>    MFC after:      3 weeks
>    Differential Revision:  https://reviews.freebsd.org/D34704
>=20
>    (cherry picked from commit =
868bf82153e8ff22f09a8860c872149e0fb6bdef)
> ---
> sys/net/if.c                   | 22 ++++++++++++++++++++--
> tests/sys/net/if_clone_test.sh | 29 +++++++++++++++++++++++++++++
> 2 files changed, 49 insertions(+), 2 deletions(-)
>=20
> diff --git a/sys/net/if.c b/sys/net/if.c
> index d4871ccbc1f7..091e9e64b99f 100644
> --- a/sys/net/if.c
> +++ b/sys/net/if.c
> @@ -548,7 +548,9 @@ vnet_if_return(const void *unused __unused)
> 	IFNET_WUNLOCK();
>=20
> 	for (int j =3D 0; j < i; j++) {
> +		sx_xlock(&ifnet_detach_sxlock);
> 		if_vmove(pending[j], pending[j]->if_home_vnet);
> +		sx_xunlock(&ifnet_detach_sxlock);
> 	}
>=20
> 	free(pending, M_IFNET);
> @@ -1393,6 +1395,8 @@ if_vmove_loan(struct thread *td, struct ifnet =
*ifp, char *ifname, int jid)
> 	bool found;
> 	bool shutdown;
>=20
> +	MPASS(ifindex_table[ifp->if_index].ife_ifnet =3D=3D ifp);
> +
> 	/* Try to find the prison within our visibility. */
> 	sx_slock(&allprison_lock);
> 	pr =3D prison_find_child(td->td_ucred->cr_prison, jid);

This particular part resulted in errors during one of my universe =
builds:

_.amd64.LINT:

/home/dim/src/stable-13/sys/net/if.c:1398:8: error: use of undeclared =
identifier 'ifindex_table'
        MPASS(ifindex_table[ifp->if_index].ife_ifnet =3D=3D ifp);
              ^

Note that it only seems to happen for the LINT kernels, though.

It appears that ifindex_table changed from a VNET define into a global
with Gleb's commit https://cgit.freebsd.org/src/commit/?id=3D80e60e236d85d=


-Dimitry


--Apple-Mail=_F9F1D825-98E6-482B-AFE0-F0F9BBB7E2D1
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
	filename=signature.asc
Content-Type: application/pgp-signature;
	name=signature.asc
Content-Description: Message signed with OpenPGP

-----BEGIN PGP SIGNATURE-----
Version: GnuPG/MacGPG2 v2.2

iF0EARECAB0WIQR6tGLSzjX8bUI5T82wXqMKLiCWowUCYpXasQAKCRCwXqMKLiCW
ozEuAKCEUzwSbJ3Tp+cVuPG7hzZINycUSQCgvSE2BYGvZYELOuQEcD6RQ+TGJhQ=
=qo3m
-----END PGP SIGNATURE-----

--Apple-Mail=_F9F1D825-98E6-482B-AFE0-F0F9BBB7E2D1--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1F175CE2-7649-4899-9CA0-8ADCE9206B6A>