[CRIU] [RFC PATCH 4/5] autofs: restore procedure introduced

[Pavel Emelyanov]

On 11/24/2015 03:54 PM, Stanislav Kinsburskiy wrote:
> 
> 
> 24.11.2015 13:24, Pavel Emelyanov пишет:
>> On 11/24/2015 02:20 PM, Stanislav Kinsburskiy wrote:
>>>
>>> 24.11.2015 11:58, Pavel Emelyanov пишет:
>>>>>>> +struct autofs_info_s {
>>>>>>> +	int pipe_fd;
>>>>>>> +	const char *mnt_path;
>>>>>>> +	struct autofs_info_s *next;
>>>>>>> +};
>>>>>>> +
>>>>>>> +struct autofs_service_s {
>>>>>>> +	int pgrp;
>>>>>>> +	struct autofs_info_s *mnt_info;
>>>>>>> +	struct autofs_service_s *next;
>>>>>>> +} *automount;
>>>>>>> +
>>>>>>> +int autofs_fixup_owner(struct autofs_info_s *info)
>>>>>>> +{
>>>>>>> +	int autofs_fd, mnt_fd, err = -1;
>>>>>>> +	struct autofs_dev_ioctl param;
>>>>>>> +
>>>>>>> +	autofs_fd = open("/dev/autofs", O_RDONLY);
>>>>>>> +	if (autofs_fd == -1) {
>>>>>>> +		pr_perror("failed to open /dev/autofs");
>>>>>>> +		return -1;
>>>>>>> +	}
>>>>>>> +
>>>>>>> +	mnt_fd = open(info->mnt_path, O_RDONLY | O_DIRECTORY);
>>>>>>> +	if (mnt_fd == -1) {
>>>>>>> +		pr_perror("failed to open %s", info->mnt_path);
>>>>>>> +		goto close_fd;
>>>>>>> +	}
>>>>>>> +
>>>>>>> +	init_autofs_dev_ioctl(&param);
>>>>>>> +	err = ioctl(mnt_fd, AUTOFS_IOC_CATATONIC, 0);
>>>>>>> +	if (err == -1) {
>>>>>>> +		pr_perror("failed to set mount point catatonic");
>>>>>>> +		goto close_mnt_fd;
>>>>>>> +
>>>>>>> +	}
>>>>>>> +
>>>>>>> +	init_autofs_dev_ioctl(&param);
>>>>>>> +	param.ioctlfd = mnt_fd;
>>>>>>> +	param.setpipefd.pipefd = info->pipe_fd;
>>>>>> The info->pipe_fd is set by autofs_mount() and the fd value is purely
>>>>>> parsed from options. Who and when creates this very fd?
>>>>> The pipe itself belongs to automount process and restored as any other fd.
>>>>> Yes, we are probably lucky, that automount doesn't close write end of
>>>>> the pipe, which is sent to the kernel.
>>>> I still don't get it. CRIU would close pipe's write end once ->open is
>>>> called, so in ->post_open the kernel part of the pipe should not be
>>>> available.
>>>>
>>>> Can you write the sequence of steps that lead to this particular place
>>>> in the code and show _where_ the write end of the pipe is (was) created?
>>>>
>>> It's very simple: user space process (automount) contains _both_ ends of
>>> the pipe. One of this ends is also used by the kernel.
>>> IOW, pipe is reopened, as any other, and write end is not closed.
>>> That's a pure luck, that automount process doesn't close write end.
>> OK, but then you should add a check for the write-end being in the process.
> 
> That's easy. But, probably, it doesn't make much sense.

It does. If you're going to restore a task _without_ that end, you'll end
up with some fake or (which is worse) wrong fd passed into autofs.

> 1) Nice fd_has_data() helper works only on read end.
> 2) We have to find read end anyway.
> 
> Is there any way to find another end of pipe without reading image file?

On dump or on restore?

>>> In case of systemd, this end is closed. And this case have to be covered
>>> as well. There should be some way, how to find the _read_ end of kernel
>>> pipe. If you know one, hint will be highly appreciated.
>> No way to get this info, kernel should be patched to at least report
>> pipe inode number.
> 
> Yes, I was also thinking about it. I'll do so for now.
> 
>>> But I suggest to fix this stuff on top of the series. While the series
>>> requires a check, that process contains both ends (like automount does).
>>>
>>> Does it suits you?
>> Yes. But I wonder how you will _safely_ distinguish which process is
>> attached to autofs mount. Doing this by pgrp only doesn't look OK.
> 
> Agreed. But if we introduce pipe inode report from autofs, then we can 
> check, that process with specified pgrp has a pipe with the same inode, 
> as kernel's control pipe.

Yup.

> In this case it should be safe from my pow.

Yes, that's the way to go. So we need a kernel patch for autofs support.

-- Pavel