Re: git: eb8dcdeac22d - main - jail: network epoch protection for IP address lists

On 26 Dec 2021, at 18:46, Gleb Smirnoff <glebius@FreeBSD.org> wrote:
> 
> The branch main has been updated by glebius:
> 
> URL: https://cgit.FreeBSD.org/src/commit/?id=eb8dcdeac22daadbf07be81d7338e14ec4cc7d7f
> 
> commit eb8dcdeac22daadbf07be81d7338e14ec4cc7d7f
> Author:     Gleb Smirnoff <glebius@FreeBSD.org>
> AuthorDate: 2021-12-26 18:45:50 +0000
> Commit:     Gleb Smirnoff <glebius@FreeBSD.org>
> CommitDate: 2021-12-26 18:45:50 +0000
> 
>    jail: network epoch protection for IP address lists
> 
>    Now struct prison has two pointers (IPv4 and IPv6) of struct
>    prison_ip type.  Each points into epoch context, address count
>    and variable size array of addresses.  These structures are
>    freed with network epoch deferred free and are not edited in
>    place, instead a new structure is allocated and set.
> 
>    While here, the change also generalizes a lot (but not enough)
>    of IPv4 and IPv6 processing. E.g. address family agnostic helpers
>    for kern_jail_set() are provided, that reduce v4-v6 copy-paste.
> 
>    The fast-path prison_check_ip[46]_locked() is also generalized
>    into prison_ip_check() that can be executed with network epoch
>    protection only.
> 
>    Reviewed by:            jamie
>    Differential revision:  https://reviews.freebsd.org/D33339
> ---
> sys/kern/kern_jail.c    | 770 +++++++++++++++++++++++++++++++-----------------
> sys/netinet/in.c        |   2 +
> sys/netinet/in_jail.c   | 139 ++-------
> sys/netinet6/in6_jail.c | 133 ++-------
> sys/sys/jail.h          |  25 +-
> 5 files changed, 572 insertions(+), 497 deletions(-)
> 
> diff --git a/sys/kern/kern_jail.c b/sys/kern/kern_jail.c
> index e505e9bf1276..f1c81d8813bd 100644
> --- a/sys/kern/kern_jail.c
> +++ b/sys/kern/kern_jail.c
> @@ -45,6 +45,7 @@ __FBSDID("$FreeBSD$");
> #include <sys/osd.h>
> #include <sys/priv.h>
> #include <sys/proc.h>
> +#include <sys/epoch.h>
> #include <sys/taskqueue.h>
> #include <sys/fcntl.h>
> #include <sys/jail.h>
> @@ -531,15 +532,407 @@ sys_jail_set(struct thread *td, struct jail_set_args *uap)
> 	return (error);
> }
> 
> +#if defined(INET) || defined(INET6)
> +typedef int prison_addr_cmp_t(const void *, const void *);
> +typedef bool prison_addr_valid_t(const void *);
> +static const struct pr_family {
> +	size_t			size;
> +	prison_addr_cmp_t	*cmp;
> +	prison_addr_valid_t	*valid;
> +	int			ip_flag;
> +} pr_families[PR_FAMILY_MAX] = {
> +#ifdef INET
> +	[PR_INET] = {
> +		.size = sizeof(struct in_addr),
> +		.cmp = prison_qcmp_v4,
> +		.valid = prison_valid_v4,
> +		.ip_flag = PR_IP4_USER,
> +	 },
> +#endif
> +#ifdef INET6
> +	[PR_INET6] = {
> +		.size = sizeof(struct in6_addr),
> +		.cmp = prison_qcmp_v6,
> +		.valid = prison_valid_v6,
> +		.ip_flag = PR_IP6_USER,
> +	},
> +#endif
> +};
> +
> +/*
> + * Network address lists (pr_addrs) allocation for jails.  The addresses
> + * are accessed locklessly by the network stack, thus need to be protected by
> + * the network epoch.
> + */
> +struct prison_ip {
> +	struct epoch_context ctx;
> +	uint32_t	ips;
> +#ifdef FUTURE_C
> +	union {
> +		struct in_addr pr_ip4[];
> +		struct in6_addr pr_ip6[];
> +	};
> +#else /* No future C :( */
> +#define	PR_IP(pip, i)	((const char *)((pip) + 1) + pr_families[af].size * (i))
> +#define	PR_IPD(pip, i)	((char *)((pip) + 1) + pr_families[af].size * (i))
> +#endif
> +};

You can make this work with a prison_ip base and prison_ipv[46] derived
structs.

As it stands this is quite gross, you’re assuming things about
alignment, and don’t even have a flexible char[] at the end to
represent the extra data.

Jess