[CRIU] [PATCH] files: fix clone_service_fd overlap handling

[Dmitry Safonov]

2018-04-17 15:44 GMT+01:00 Kirill Tkhai <ktkhai at virtuozzo.com>:
> On 17.04.2018 17:07, Pavel Tikhomirov wrote:
>> Though LOG_FD_OFF < IMG_FD_OFF, get_service_fd(LOG_FD_OFF) is > than
>> get_service_fd(IMG_FD_OFF), see __get_service_fd, so the check here
>> should be twisted. Also add bug_on to track possible __get_service_fd
>> change which can break these check again.
>>
>> We have a problem when USERNSD_SK replaces LOG_FD_OFF, latter when
>> writing to log, instead we actually send crazy commands to usernsd,
>> which failes to handle them and BUG or crash.
>>
>> https://jira.sw.ru/browse/PSBM-83472
>>
>> Also we had similar problem when __userns_call received bad repsonse,
>> likely it has the same background.
>>
>> https://api.travis-ci.org/v3/job/352164661/log.txt
>>
>> fixes commit 129bb14611c3 ("files: Prepare clone_service_fd() for
>> overlaping ranges.")
>>
>> Signed-off-by: Pavel Tikhomirov <ptikhomirov at virtuozzo.com>
>> ---
>>  criu/util.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/criu/util.c b/criu/util.c
>> index b19bf5175..4af542a4f 100644
>> --- a/criu/util.c
>> +++ b/criu/util.c
>> @@ -617,7 +617,8 @@ int clone_service_fd(struct pstree_item *me)
>>               return 0;
>>
>>       /* Dup sfds in memmove() style: they may overlap */
>> -     if (get_service_fd(LOG_FD_OFF) > new_base - LOG_FD_OFF - SERVICE_FD_MAX * id)
>
> Good catch!
>
>> +     BUG_ON(get_service_fd(LOG_FD_OFF) < get_service_fd(IMG_FD_OFF));
>
> I don't think we should check this in runtime. If we need this, this should be made
> once in one of the init functions.

I might be wrong, but it looks to me the same as:
BUILD_BUG_ON(LOG_FD_OFF > IMG_FD_OFF)

so we can omit runtime penalty and keep it in place if I'm not mistaken.
But maybe you want to check the blackbox of get_service_fd(), not
what it implies.

-- 
             Dmitry