[CRIU] [PATCH 01/11] cr-check: Add check for mremap() of special mappings

Dmitry Safonov 0x7f454c46 at gmail.com
Mon May 27 17:21:23 MSK 2019


On 5/25/19 6:42 AM, Andrei Vagin wrote:
> On Wed, May 22, 2019 at 07:18:15PM +0100, Dmitry Safonov wrote:
[..]
>> +static void check_special_mapping_mremap_child(struct special_mapping *vmas,
>> +					       size_t nr)
>> +{
>> +	size_t i, parking_size = 0;
>> +	void *parking_lot;
>> +	pid_t self = getpid();
>> +
>> +	for (i = 0; i < nr; i++) {
>> +		if (vmas[i].addr != MAP_FAILED)
>> +			parking_size += vmas[i].size;
>> +	}
>> +
> 
> Could you write a comment why we need to handle SIGUSR1 here?

Sure.

> 
>> +	if (signal(SIGUSR1, dummy_sighandler) == SIG_ERR) {
>> +		pr_perror("signal() failed");
>> +		exit(1);
>> +	}
>> +
>> +	parking_lot = mmap(NULL, parking_size, PROT_NONE,
>> +			   MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
>> +	if (parking_lot == MAP_FAILED) {
>> +		pr_perror("mmap(%zu) failed", parking_size);
>> +		exit(1);
>> +	}
>> +
>> +	for (i = 0; i < nr; i++) {
>> +		unsigned long ret;
>> +
>> +		if (vmas[i].addr == MAP_FAILED)
>> +			continue;
>> +
>> +		ret = syscall(__NR_mremap, (unsigned long)vmas[i].addr,
>> +			      vmas[i].size, vmas[i].size,
>> +			      MREMAP_FIXED | MREMAP_MAYMOVE,
>> +			      (unsigned long)parking_lot);
>> +		if (ret != (unsigned long)parking_lot)
> 
> if it fails, we probably can log this error

It would be ugly at this place as it needs raw sys_write().
Probably, it will be prettier if I just print exit status in the parent.

[..]
>> +	child = fork();
>> +	if (child < 0) {
>> +		pr_perror("%s(): failed to fork()", __func__);
>> +		return -1;
>> +	}
>> +
>> +	if (child == 0) {
>> +		check_special_mapping_mremap_child(special_vmas, vmas_nr);
> 		exit(1); /* unreachable */
> 	}
>> +
>> +	if (waitpid(child, &stat, 0) != child) {
>> +		pr_err("Failed to wait for special mapping mremap() test\n");
>> +		kill(child, SIGKILL);
> if waitpid failed, we probably doesn't have this child, so you would
> prefer to not kill a process with this pid.

Yeah, I guess I'll convert it to ECHILD check and exit without shooting
somebody.

-- 
          Dima


More information about the CRIU mailing list