[Devel] Re: [PATCH 1/3] Checkpoint/restart epoll sets

[Oren Laadan]

Serge E. Hallyn wrote:
> Quoting Matt Helsley (matthltc at us.ibm.com):
>> @@ -1226,35 +1242,18 @@ SYSCALL_DEFINE1(epoll_create, int, size)
>>   * the eventpoll file that enables the insertion/removal/change of
>>   * file descriptors inside the interest set.

[...]

>> +	if (h->h.type != CKPT_HDR_FILE ||
>> +	    h->h.len  != sizeof(*h) ||
>> +	    h->f_type != CKPT_FILE_EPOLL)
>> +		return ERR_PTR(-EINVAL);
>> +
>> +	epfd = sys_epoll_create1(h->f_flags & EPOLL_CLOEXEC);
>> +	if (epfd < 0)
>> +		return ERR_PTR(epfd);
>> +	epfile = fget(epfd);
>> +	sys_close(epfd); /* harmless even if an error occured */
>> +	BUG_ON(!epfile);
> 
> Would perhaps return ERR_PTR(-ENOENT) be nicer?  (And maybe safer - I'm
> not quite clear on under which arches BUG_ON does nothing).

Serge is right: malicious userspace could fork the restarting tasks
to all share fdtable with a non-restarting task, and that other task
could close the fd ...

I'll write a patch that ensures that the root task doesn't share
anything with its parent (coordinator).

But the race still exists for self-restart. So I'd do -EBUSY here
instead.

Oren.

_______________________________________________
Containers mailing list
Containers at lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers