Re: [Xen-devel] [PATCH XEN v6 25/32] tools/libs/gnttab: Extensive updates to API documentation.

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 09/12/15 13:00, Ian Campbell wrote:
> On Wed, 2015-12-09 at 12:41 +0000, Wei Liu wrote:
>>> + */
>>> +
>>> +/*
>>>   * Grant Table Interface (making use of grants from other domains)
>>>   */
>>>  
>>>  typedef struct xengntdev_handle xengnttab_handle;
>>>  
>>>  /*
>>> - * Note:
>>> - * After fork a child process must not use any opened xc gnttab
>>> - * handle inherited from their parent. They must open a new handle if
>>> - * they want to interact with xc.
>>> + * Returns a handle onto the grant table driver.  Logs errors.
>>> + *
>>> + * Note: After fork a child process must not use any opened gnttab
>>> + * handle inherited from their parent, more access any grant mapped
>>> + * areas associated with that handle.
>>> + *
>> And this could use the same cloexec trick as you do in other patch for
>> privcmd.
> I think this statement "do not use the handle" still stands regardless of
> the underlying fd being cloexec'd.
>
> However, this did make me think about a comment elsewhere, specifically the
> various instances of:
>  * This is the only function which may be safely called on a
>  * xen<foo>_handle in a child after a fork.
> for several xen<foo>_close() functions.
>
> This is not really true if the fd is cloexec, since then _close() will
> either fail (EBADF) or, worse, close some other freshly opened file
> descriptor.
>
> There seems to be two choices here, either require all osdep backends to
> make their fds O_CLOEXEC (which might involve tolerating a racy
> fcntl+FD_CLOEXEC pattern after open on some platforms) or don't set
> O_CLOEXEC ever and declare that the application is responsible for closing
> after fork, and for taking care of the corner cases themselves in
> multithreaded applications.
>
> The former seems friendlier to me, even if some platforms need to use
> FD_CLOEXEC.
>
> Hrm, maybe I can extend the atfork trick to cover the open+fcntl bits.
> Hopefully there is no issue with using pthreads from each of the affected
> libraries.
>
> So, I think the advice in the comment would then be:
>
>     If you fork and then exec then you must not (and need not) call _close()
>     or any other function on the handle.
>
>     If you fork but do not exec then it is permissible to call _close(), but
>     it is not permissible to call any other function on the handle.
>
> Need to think about that wording.

This is risky.  What if $FOO_open() allocated more resource than just
the CLOEXEC fd?

An _open() call must be matched with a _close() call.

In the case of fork but no exec, there should be a _close() call in both
the parent and child, to free other resources.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel