[CRIU] [PATCH] compel: compel_emergency_sigframe() helper introduced

[Stanislav Kinsburskiy]

27.03.2017 17:13, Dmitry Safonov пишет:
> 2017-03-27 17:25 GMT+03:00 Stanislav Kinsburskiy <skinsbursky at virtuozzo.com>:
>>
>> 27.03.2017 16:07, Dmitry Safonov пишет:
>>> Yes, but fini_sigreturn() at this moment is called only for parasite-based
>>> infection. While the patch exports API for non-parasite case of infection.
>>> So, as I've asked before, can we complement it with usage of the formed
>>> sigframe in the case of non-parasite infection?
>>> It may be just a helper that has inside compel_syscall() with
>>> rt_sigreturn.
>>> I mean, if anyone is going to use API for forming sigframe, he will
>>> have to write such helper. Maybe it's better to provide one for integrity?
>>>
>> It's not that simple.
>> User has to provide address *within* process address space, where sigframe
>> must be placed, *to* sigframe creation routine.
>> IOW, one need:
>> 1) Some service mapping within process (doesn't exist in non-parasite case
>> of infection), which he can use to store sigframe
>> 2) Some way to call sigreturn from *within* the process (doesn't exist in
>> non-parasite case of infection)
>> 3) Calling to "rt_sigreturn" needs this sigframe address.
>>
>> Thus, from my POW, libcompel can to provide this helper "as is".
>> Or this facility has to be significantly improved by hiding all this remote
>> mapping creation, sigrame construction and rt_sigreturn call to some kind of
>> helpers like below:
>>
>> int compel_put_some_straw(struct parasite_ctl *ctl);
>>
>> and paired (static, inlined, whatever else to prevent linking with
>> libcompel):
>>
>> int compel_fall_from_the tree(void);
>>
> Well, I would vote for something like:
>
> compel_remote_mmap(struct parasite_ctl *ctl, unsigned long addr)
> /* actually renamed compel_unmap() */
> compel_remote_unmap(struct parasite_ctl *ctl, unsigned long addr)
> compel_sigframe_prepare(struct rt_sigframe *raddr,
>     user_regs_struct_t *regs, k_rtsigset_t* sig)
> compel_sigframe_sigreturn(struct rt_sigframe *raddr)
>
> Maybe the naming could be improved.
> This way we would have complete API for preparing sigframes for any
> register set, blocked signals mask and for sigreturning to constructed
> frames. All without parasite's help.
>
> Otherwise the original patch exports API for preparing sigframe only
> at the point where an application was stopped and leaves reader's
> exercise what to do with this sigframe afterward.
>
> What do you think, is it convenient API for your project?
> I don't insist, but that looks logical to me avoiding inchoate APIs.
>

Well, you suggest to introduce 4 helpers. And add them to API.
I suspect (almost sure) the following will happen:
1) Someone (or "The one", if you wish) will come and say, that names are 
bad and unacceptable.
2) Then he will come once more and say, that "compel_remote_mmap/unmap" 
are stupid wrappers around compel_syscall(), which is ugly and unacceptable.

And even if you somehow push this stuff, it won't be really valuable.
Because there is no "fool protection" and user can easily shoot in his 
foot by passing any wrong address to compel_sigframe_prepare() as raddr,
which will crash the tracee upon compel_sigframe_sigreturn() call.

And one more thing to clarify, what you actually suggested: 
compel_sigframe_sigreturn() (or at least part of its logic) has to be 
executed in tracee.
I see two ways to do this:
1) Link tracee with libcompel to take rt_sigreturn helper from there. 
"Link" here means load the library into tracee.
2) Be able to place some "rt_sigreturn" code to an arbitrary address 
within the process, and then jump to it.

So, yet again, I believe, that this API either has to be difficult to 
use (for those, who understand, what they are doing) or all the logic 
has to be hidden inside the library to prevent user from shooting 
himself in the foot.