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

[Pavel Tikhomirov]

On 07/15/2016 08:36 PM, Pavel Emelyanov wrote:
> 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.

Ok but that mean criu will make F23 crash as latest kernel there is 
4.5.7-202.fc23. Men from redhat seem do not want to fix it in F23.

>
>> 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();
>>
>>
>

-- 
Best regards, Tikhomirov Pavel
Software Developer, Virtuozzo.