[CRIU] [PATCH] security: check_ids - return true if [se]?[ug]id is the same as task id

[Pavel Emelyanov]

On 05/19/2014 06:17 PM, Ruslan Kuprieiev wrote:
> On 17.05.2014 12:00, Andrew Vagin wrote:
>> On Fri, May 16, 2014 at 05:54:55PM +0300, Ruslan Kuprieiev wrote:
>>> Currently there are typos in check_ids, so one can't pass this check,
>>> unless (u/g)id == e(g/u)id == s(g/u)id == task_(g/u)id.
> 
> OMG, I mixed task and caller id's in this description!

The checks you're fixing prevent from creating images with "bad"
code and restore them into siud-ed process. What problem are we
trying to resolve? If I get it right it is -- task executes a
suid-ed binary belonging to some other user, then we checkpoint
it, then try to restore and fail. Is that correct?

>>> Signed-off-by: Ruslan Kuprieiev <kupruser at gmail.com>
>>> ---
>>>   security.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/security.c b/security.c
>>> index d4b4230..dc90208 100644
>>> --- a/security.c
>>> +++ b/security.c
>>> @@ -28,7 +28,7 @@ static bool check_ids(unsigned int crid, unsigned int rid,
>>> unsigned int eid, uns
>>>   {
>>>       if (crid == 0)
>>>           return true;
>>> -    if (crid == rid && crid == eid && crid == sid)
>>> +    if (crid == rid || crid == eid || crid == sid)
>> I have thought a bit more and now I am not sure about this. Could you explain why this is correct?
> 
> Hm... Yes, you are right. Well, crid == rid || crid == sid we get from the
> statement, that we should be able to dump task, if we can kill it.
> And crid == eid looks wrong.
> 
>>>           return true;
>>>
>>>       pr_err("UID/GID mismatch %u != (%u,%u,%u)\n", crid, rid, eid, sid);
>>> -- 
>>> 1.8.1.2
>>>
>>> _______________________________________________
>>> CRIU mailing list
>>> CRIU at openvz.org
>>> https://lists.openvz.org/mailman/listinfo/criu
> 
>