Re: [Qemu-devel] [PULL 4/5] usb-mtp: Introduce write support for MTP objects

[Bandan Das <bsd@redhat.com>]

Eric Blake <eblake@redhat.com> writes:

> On 02/20/2018 09:28 AM, Gerd Hoffmann wrote:
>> From: Bandan Das <bsd@redhat.com>
>>
>> Allow write operations on behalf of the initiator. The
>> precursor to write is the sending of the write metadata
>> that consists of the ObjectInfo dataset. This patch introduces
>> a flag that is set when the responder is ready to receive
>> write data based on a previous SendObjectInfo operation by
>> the initiator (The SendObjectInfo implementation is in a
>> later patch)
>>
>> Signed-off-by: Bandan Das <bsd@redhat.com>
>> Message-id: 20180215231129.14710-5-bsd@redhat.com
>> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>> ---
>>   hw/usb/dev-mtp.c | 159 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>   1 file changed, 157 insertions(+), 2 deletions(-)
>>
>
>> @@ -1472,12 +1492,133 @@ static void usb_mtp_cancel_packet(USBDevice *dev, USBPacket *p)
>>       fprintf(stderr, "%s\n", __func__);
>>   }
>>   +mode_t getumask(void)
>> +{
>> +    mode_t mask = umask(0);
>> +    umask(mask);
>> +    return mask;
>> +}
>
> This is dangerous.  'man getumask' on my Fedora machine states:

Thanks for the pointer, Eric. Indeed, this doesn't look right.
I am inclined to just set the default
permission of created files to 0644 for the mtp share
and not implement a thread safe getumask.

Bandan

> CONFORMING TO
>        This is a vaporware GNU extension.
>
> NOTES
>        This  function is documented in the glibc manual, but, as at
> glibc ver‐
>        sion 2.24, it is not implemented on Linux.  (See umask(2) for a
> thread-
>        safe method of discovering a process's umask.)
>
>
> and 'man 2 umask' concurs:
>
>        It  is  impossible to use umask() to fetch a process's umask
> without at
>        the same time changing it.  A second call  to  umask()  would
> then  be
>        needed  to restore the umask.  The nonatomicity of these two
> steps pro‐
>        vides the potential for races in multithreaded programs.
>
> It is ONLY safe to grab umask() prior to spawning threads, cache that
> value, and refer to the cache at all later points.