[CRIU] [PATCH v3 2/5] net/sysctl: add sysctl_igmp_link_local_mcast_reports_safe check

[Pavel Emelyanov]

On 07/14/2016 04:51 PM, Pavel Tikhomirov wrote:
> In Linux v4.3 commit df2cf4a78e48 ("IGMP: Inhibit reports for local
> multicast groups") sysctl igmp_link_local_mcast_reports was introduced
> in ipv4_net_table.
> 
> And in ipv4_net_table it's data was initialized to point on
> sysctl_igmp_llm_reports variable. That was so before commit
> 87a8a2ae65b7 ("igmp: Namespaceify igmp_llm_reports sysctl knob").
> 
> So next it's data pointer is shifted to the offset of current
> netnamespace relative to init_net in ipv4_sysctl_init_net function.
> But that is completely wrong if variable is not net-namespaced, so we
> get random kernel address and can write/read to/from it one int, that
> can lead to memory corruption and crashes in random places in kernel.
> 
> So conclusion is: we can not touch
> /proc/sys/net/ipv4/igmp_link_local_mcast_reports in v4.3-v4.5 between
> those two patches.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1352177

This looks like a workaround for a known and fixed bug in the kernel
which is not criu-specific. Is it? If so, I don't see much point in
the patch, if someone drives into this problem, we just suggest one
to add the fixing patch to kernel.

> https://jira.sw.ru/browse/PSBM-48397
> 
> Signed-off-by: Pavel Tikhomirov <ptikhomirov at virtuozzo.com>
> ---
>  criu/include/kerndat.h |  1 +
>  criu/kerndat.c         | 40 ++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 41 insertions(+)
> 
> diff --git a/criu/include/kerndat.h b/criu/include/kerndat.h
> index 0a5cd4b..9bd1625 100644
> --- a/criu/include/kerndat.h
> +++ b/criu/include/kerndat.h
> @@ -37,6 +37,7 @@ struct kerndat_s {
>  	bool has_compat_sigreturn;
>  	enum pagemap_func pmap;
>  	unsigned int has_xtlocks;
> +	bool sysctl_igmp_link_local_mcast_reports_safe;
>  };
>  
>  extern struct kerndat_s kdat;
> diff --git a/criu/kerndat.c b/criu/kerndat.c
> index 04a355b..2a02678 100644
> --- a/criu/kerndat.c
> +++ b/criu/kerndat.c
> @@ -7,6 +7,7 @@
>  #include <sys/mman.h>
>  #include <errno.h>
>  #include <sys/syscall.h>
> +#include <sys/utsname.h>
>  
>  #include "log.h"
>  #include "bug.h"
> @@ -460,6 +461,41 @@ static int kerndat_compat_restore(void)
>  	return 0;
>  }
>  
> +static int kerndat_sysctl_igmp_link_local_mcast_reports_safe(void)
> +{
> +	int ret;
> +	struct utsname buf;
> +	char smajor[10], sminor[10];
> +	int major, minor;
> +
> +	kdat.sysctl_igmp_link_local_mcast_reports_safe = false;
> +
> +	ret = uname(&buf);
> +	if (ret) {
> +		pr_perror("Failed uname syscall");
> +		return 1;
> +	}
> +
> +	ret = sscanf(buf.release, "%[^'.'].%[^'.']", smajor, sminor);
> +	if (ret != 2) {
> +		pr_perror("Failed to parse major/minor from uname");
> +		return 1;
> +	}
> +
> +	major = atoi(smajor);
> +	minor = atoi(sminor);
> +
> +	if (!(major == 4 && minor >= 3 && minor <= 5))
> +		/*
> +		 * In Linux v4.3 commit df2cf4a78e48 ("IGMP: Inhibit reports for local multicast groups")
> +		 * buggy sysctl is added, it allows to write at random memory offsets from netns
> +		 * In Linux v4.6 commit 87a8a2ae65b7 ("igmp: Namespaceify igmp_llm_reports sysctl knob") it is fixed.
> +		 */
> +		kdat.sysctl_igmp_link_local_mcast_reports_safe = true;
> +
> +	return 0;
> +}
> +
>  int kerndat_init(void)
>  {
>  	int ret;
> @@ -485,6 +521,8 @@ int kerndat_init(void)
>  		ret = kerndat_iptables_has_xtlocks();
>  	if (!ret)
>  		ret = kerndat_compat_restore();
> +	if (!ret)
> +		ret = kerndat_sysctl_igmp_link_local_mcast_reports_safe();
>  
>  	kerndat_lsm();
>  
> @@ -516,6 +554,8 @@ int kerndat_init_rst(void)
>  		ret = kerndat_iptables_has_xtlocks();
>  	if (!ret)
>  		ret = kerndat_compat_restore();
> +	if (!ret)
> +		ret = kerndat_sysctl_igmp_link_local_mcast_reports_safe();
>  
>  	kerndat_lsm();
>  
>