linux-man.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] system.3: Indicate MT-Unsafe
@ 2020-10-06 16:15 Nate Karstens
  2020-10-06 17:26 ` Adhemerval Zanella
  0 siblings, 1 reply; 6+ messages in thread
From: Nate Karstens @ 2020-10-06 16:15 UTC (permalink / raw)
  To: mtk.manpages; +Cc: linux-man, libc-alpha, willy, Nate Karstens

The fact that system(3) does not support pthread_atfork(3) also means
that it is not thread safe. See the discussion for the proposal of a
close-on-fork flag in the 2020 April and May timeframe, especially:

https://lkml.org/lkml/2020/5/15/1067

Signed-off-by: Nate Karstens <nate.karstens@garmin.com>
---
 man3/system.3 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/man3/system.3 b/man3/system.3
index aef40417a..8730fabd3 100644
--- a/man3/system.3
+++ b/man3/system.3
@@ -127,7 +127,7 @@ l l l.
 Interface	Attribute	Value
 T{
 .BR system ()
-T}	Thread safety	MT-Safe
+T}	Thread safety	MT-Unsafe
 .TE
 .SH CONFORMING TO
 POSIX.1-2001, POSIX.1-2008, C89, C99.
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] system.3: Indicate MT-Unsafe
  2020-10-06 16:15 [PATCH] system.3: Indicate MT-Unsafe Nate Karstens
@ 2020-10-06 17:26 ` Adhemerval Zanella
  2020-10-07 14:35   ` Karstens, Nate
  0 siblings, 1 reply; 6+ messages in thread
From: Adhemerval Zanella @ 2020-10-06 17:26 UTC (permalink / raw)
  To: Nate Karstens, mtk.manpages; +Cc: linux-man, libc-alpha, willy



On 06/10/2020 13:15, Nate Karstens via Libc-alpha wrote:
> The fact that system(3) does not support pthread_atfork(3) also means
> that it is not thread safe. See the discussion for the proposal of a
> close-on-fork flag in the 2020 April and May timeframe, especially:
> 
> https://lkml.org/lkml/2020/5/15/1067
> 
> Signed-off-by: Nate Karstens <nate.karstens@garmin.com>

Not sure if man pages characterizes file descriptor leak as mt-unsafe, at
least we don't have this concept on glibc manual.  In fact, I think adding
a MT-Unsafe mark to this potentially make any libc call that is not atomic
potentially MT-Unsafe, either when they do not concurrent trigger race
issues regarding memory semantic. At least I think it should add a 'race'
mark to indicate what exactly is MT-unsafe (as for other implementations).

> ---
>  man3/system.3 | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/man3/system.3 b/man3/system.3
> index aef40417a..8730fabd3 100644
> --- a/man3/system.3
> +++ b/man3/system.3
> @@ -127,7 +127,7 @@ l l l.
>  Interface	Attribute	Value
>  T{
>  .BR system ()
> -T}	Thread safety	MT-Safe
> +T}	Thread safety	MT-Unsafe
>  .TE
>  .SH CONFORMING TO
>  POSIX.1-2001, POSIX.1-2008, C89, C99.
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH] system.3: Indicate MT-Unsafe
  2020-10-06 17:26 ` Adhemerval Zanella
@ 2020-10-07 14:35   ` Karstens, Nate
  2020-10-07 18:06     ` Adhemerval Zanella
  0 siblings, 1 reply; 6+ messages in thread
From: Karstens, Nate @ 2020-10-07 14:35 UTC (permalink / raw)
  To: Adhemerval Zanella, mtk.manpages; +Cc: linux-man, libc-alpha, willy

I'm fine with adding a "race" qualifier. Do you have any ideas on the type of race? I didn't see anything in the other man-pages that jumped out as being correct.

Thanks,

Nate

-----Original Message-----
From: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Sent: Tuesday, October 06, 2020 12:26
To: Karstens, Nate <Nate.Karstens@garmin.com>; mtk.manpages@gmail.com
Cc: linux-man@vger.kernel.org; libc-alpha@sourceware.org; willy@infradead.org
Subject: Re: [PATCH] system.3: Indicate MT-Unsafe

CAUTION - EXTERNAL EMAIL: Do not click any links or open any attachments unless you trust the sender and know the content is safe.


On 06/10/2020 13:15, Nate Karstens via Libc-alpha wrote:
> The fact that system(3) does not support pthread_atfork(3) also means
> that it is not thread safe. See the discussion for the proposal of a
> close-on-fork flag in the 2020 April and May timeframe, especially:
>
> https://urldefense.com/v3/__https://lkml.org/lkml/2020/5/15/1067__;!!E
> Jc4YC3iFmQ!D9YVAE760hT-YFoOT14KmIu4y2cjQb8ZflVgpX-3rxgBF2WvxyATUeQogZF
> Ffv2sIQ$
>
> Signed-off-by: Nate Karstens <nate.karstens@garmin.com>

Not sure if man pages characterizes file descriptor leak as mt-unsafe, at least we don't have this concept on glibc manual.  In fact, I think adding a MT-Unsafe mark to this potentially make any libc call that is not atomic potentially MT-Unsafe, either when they do not concurrent trigger race issues regarding memory semantic. At least I think it should add a 'race'
mark to indicate what exactly is MT-unsafe (as for other implementations).

> ---
>  man3/system.3 | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/man3/system.3 b/man3/system.3 index aef40417a..8730fabd3
> 100644
> --- a/man3/system.3
> +++ b/man3/system.3
> @@ -127,7 +127,7 @@ l l l.
>  Interface    Attribute       Value
>  T{
>  .BR system ()
> -T}   Thread safety   MT-Safe
> +T}   Thread safety   MT-Unsafe
>  .TE
>  .SH CONFORMING TO
>  POSIX.1-2001, POSIX.1-2008, C89, C99.
>

________________________________

CONFIDENTIALITY NOTICE: This email and any attachments are for the sole use of the intended recipient(s) and contain information that may be Garmin confidential and/or Garmin legally privileged. If you have received this email in error, please notify the sender by reply email and delete the message. Any disclosure, copying, distribution or use of this communication (including attachments) by someone other than the intended recipient is prohibited. Thank you.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] system.3: Indicate MT-Unsafe
  2020-10-07 14:35   ` Karstens, Nate
@ 2020-10-07 18:06     ` Adhemerval Zanella
  2020-10-27  7:08       ` Michael Kerrisk (man-pages)
  0 siblings, 1 reply; 6+ messages in thread
From: Adhemerval Zanella @ 2020-10-07 18:06 UTC (permalink / raw)
  To: Karstens, Nate, mtk.manpages; +Cc: linux-man, libc-alpha, willy

But calling system does not really incur in a race w.r.t the resources
of the caller itself and least glibc does handle concurrent sigactions 
calls and thread cancellation (by reaping the created process). I am
not sure about other libc implementation though.

The file leakage will be only for the spawn program itself and although 
it is might characterize as unsafe to call 'system' in multithread 
environment I don't think this characterize as MT-unsafe (and with 
FD_CLOEXEC/O_CLOEXEC system is indeed safe in this regarding).

So maybe document using a different markup to make it explicit?

On 07/10/2020 11:35, Karstens, Nate wrote:
> I'm fine with adding a "race" qualifier. Do you have any ideas on the type of race? I didn't see anything in the other man-pages that jumped out as being correct.
> 
> Thanks,
> 
> Nate
> 
> -----Original Message-----
> From: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> Sent: Tuesday, October 06, 2020 12:26
> To: Karstens, Nate <Nate.Karstens@garmin.com>; mtk.manpages@gmail.com
> Cc: linux-man@vger.kernel.org; libc-alpha@sourceware.org; willy@infradead.org
> Subject: Re: [PATCH] system.3: Indicate MT-Unsafe
> 
> CAUTION - EXTERNAL EMAIL: Do not click any links or open any attachments unless you trust the sender and know the content is safe.
> 
> 
> On 06/10/2020 13:15, Nate Karstens via Libc-alpha wrote:
>> The fact that system(3) does not support pthread_atfork(3) also means
>> that it is not thread safe. See the discussion for the proposal of a
>> close-on-fork flag in the 2020 April and May timeframe, especially:
>>
>> https://urldefense.com/v3/__https://lkml.org/lkml/2020/5/15/1067__;!!E
>> Jc4YC3iFmQ!D9YVAE760hT-YFoOT14KmIu4y2cjQb8ZflVgpX-3rxgBF2WvxyATUeQogZF
>> Ffv2sIQ$
>>
>> Signed-off-by: Nate Karstens <nate.karstens@garmin.com>
> 
> Not sure if man pages characterizes file descriptor leak as mt-unsafe, at least we don't have this concept on glibc manual.  In fact, I think adding a MT-Unsafe mark to this potentially make any libc call that is not atomic potentially MT-Unsafe, either when they do not concurrent trigger race issues regarding memory semantic. At least I think it should add a 'race'
> mark to indicate what exactly is MT-unsafe (as for other implementations).
> 
>> ---
>>  man3/system.3 | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/man3/system.3 b/man3/system.3 index aef40417a..8730fabd3
>> 100644
>> --- a/man3/system.3
>> +++ b/man3/system.3
>> @@ -127,7 +127,7 @@ l l l.
>>  Interface    Attribute       Value
>>  T{
>>  .BR system ()
>> -T}   Thread safety   MT-Safe
>> +T}   Thread safety   MT-Unsafe
>>  .TE
>>  .SH CONFORMING TO
>>  POSIX.1-2001, POSIX.1-2008, C89, C99.
>>
> 
> ________________________________
> 
> CONFIDENTIALITY NOTICE: This email and any attachments are for the sole use of the intended recipient(s) and contain information that may be Garmin confidential and/or Garmin legally privileged. If you have received this email in error, please notify the sender by reply email and delete the message. Any disclosure, copying, distribution or use of this communication (including attachments) by someone other than the intended recipient is prohibited. Thank you.
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] system.3: Indicate MT-Unsafe
  2020-10-07 18:06     ` Adhemerval Zanella
@ 2020-10-27  7:08       ` Michael Kerrisk (man-pages)
  2020-10-30 20:59         ` Karstens, Nate
  0 siblings, 1 reply; 6+ messages in thread
From: Michael Kerrisk (man-pages) @ 2020-10-27  7:08 UTC (permalink / raw)
  To: Adhemerval Zanella, Karstens, Nate
  Cc: mtk.manpages, linux-man, libc-alpha, willy

On 10/7/20 8:06 PM, Adhemerval Zanella wrote:
> But calling system does not really incur in a race w.r.t the resources
> of the caller itself and least glibc does handle concurrent sigactions 
> calls and thread cancellation (by reaping the created process). I am
> not sure about other libc implementation though.
> 
> The file leakage will be only for the spawn program itself and although 
> it is might characterize as unsafe to call 'system' in multithread 
> environment I don't think this characterize as MT-unsafe (and with 
> FD_CLOEXEC/O_CLOEXEC system is indeed safe in this regarding).
> 
> So maybe document using a different markup to make it explicit?

Any further conclusions out of this?

Thanks,

Michael

> On 07/10/2020 11:35, Karstens, Nate wrote:
>> I'm fine with adding a "race" qualifier. Do you have any ideas on the type of race? I didn't see anything in the other man-pages that jumped out as being correct.
>>
>> Thanks,
>>
>> Nate
>>
>> -----Original Message-----
>> From: Adhemerval Zanella <adhemerval.zanella@linaro.org>
>> Sent: Tuesday, October 06, 2020 12:26
>> To: Karstens, Nate <Nate.Karstens@garmin.com>; mtk.manpages@gmail.com
>> Cc: linux-man@vger.kernel.org; libc-alpha@sourceware.org; willy@infradead.org
>> Subject: Re: [PATCH] system.3: Indicate MT-Unsafe
>>
>> CAUTION - EXTERNAL EMAIL: Do not click any links or open any attachments unless you trust the sender and know the content is safe.
>>
>>
>> On 06/10/2020 13:15, Nate Karstens via Libc-alpha wrote:
>>> The fact that system(3) does not support pthread_atfork(3) also means
>>> that it is not thread safe. See the discussion for the proposal of a
>>> close-on-fork flag in the 2020 April and May timeframe, especially:
>>>
>>> https://urldefense.com/v3/__https://lkml.org/lkml/2020/5/15/1067__;!!E
>>> Jc4YC3iFmQ!D9YVAE760hT-YFoOT14KmIu4y2cjQb8ZflVgpX-3rxgBF2WvxyATUeQogZF
>>> Ffv2sIQ$
>>>
>>> Signed-off-by: Nate Karstens <nate.karstens@garmin.com>
>>
>> Not sure if man pages characterizes file descriptor leak as mt-unsafe, at least we don't have this concept on glibc manual.  In fact, I think adding a MT-Unsafe mark to this potentially make any libc call that is not atomic potentially MT-Unsafe, either when they do not concurrent trigger race issues regarding memory semantic. At least I think it should add a 'race'
>> mark to indicate what exactly is MT-unsafe (as for other implementations).
>>
>>> ---
>>>  man3/system.3 | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/man3/system.3 b/man3/system.3 index aef40417a..8730fabd3
>>> 100644
>>> --- a/man3/system.3
>>> +++ b/man3/system.3
>>> @@ -127,7 +127,7 @@ l l l.
>>>  Interface    Attribute       Value
>>>  T{
>>>  .BR system ()
>>> -T}   Thread safety   MT-Safe
>>> +T}   Thread safety   MT-Unsafe
>>>  .TE
>>>  .SH CONFORMING TO
>>>  POSIX.1-2001, POSIX.1-2008, C89, C99.
>>>
>>
>> ________________________________
>>
>> CONFIDENTIALITY NOTICE: This email and any attachments are for the sole use of the intended recipient(s) and contain information that may be Garmin confidential and/or Garmin legally privileged. If you have received this email in error, please notify the sender by reply email and delete the message. Any disclosure, copying, distribution or use of this communication (including attachments) by someone other than the intended recipient is prohibited. Thank you.
>>


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH] system.3: Indicate MT-Unsafe
  2020-10-27  7:08       ` Michael Kerrisk (man-pages)
@ 2020-10-30 20:59         ` Karstens, Nate
  0 siblings, 0 replies; 6+ messages in thread
From: Karstens, Nate @ 2020-10-30 20:59 UTC (permalink / raw)
  To: Michael Kerrisk (man-pages), Adhemerval Zanella
  Cc: linux-man, libc-alpha, willy

I don't know, my interpretation of what happens is that the race is related to the resources of the caller. Inside of system() the program essentially forked, increasing the OS's reference count of those file descriptors. At the same time, another thread in the caller attempts to close & reopen those resources, but fails because it cannot get exclusive access. In a single-threaded process this would not happen because system() doesn't return until the child process is complete. I think most people will check the thread safety attribute and so changing this will be the most obvious way to indicate a potential problem.

But maybe there is some subtlety I'm not picking up on, I'm open to other ideas on how best to communicate this.

Thanks,

Nate

-----Original Message-----
From: Michael Kerrisk (man-pages) <mtk.manpages@gmail.com> 
Sent: Tuesday, October 27, 2020 02:08
To: Adhemerval Zanella <adhemerval.zanella@linaro.org>; Karstens, Nate <Nate.Karstens@garmin.com>
Cc: mtk.manpages@gmail.com; linux-man@vger.kernel.org; libc-alpha@sourceware.org; willy@infradead.org
Subject: Re: [PATCH] system.3: Indicate MT-Unsafe

On 10/7/20 8:06 PM, Adhemerval Zanella wrote:
> But calling system does not really incur in a race w.r.t the resources 
> of the caller itself and least glibc does handle concurrent sigactions 
> calls and thread cancellation (by reaping the created process). I am 
> not sure about other libc implementation though.
> 
> The file leakage will be only for the spawn program itself and 
> although it is might characterize as unsafe to call 'system' in 
> multithread environment I don't think this characterize as MT-unsafe 
> (and with FD_CLOEXEC/O_CLOEXEC system is indeed safe in this regarding).
> 
> So maybe document using a different markup to make it explicit?

Any further conclusions out of this?

Thanks,

Michael

> On 07/10/2020 11:35, Karstens, Nate wrote:
>> I'm fine with adding a "race" qualifier. Do you have any ideas on the type of race? I didn't see anything in the other man-pages that jumped out as being correct.
>>
>> Thanks,
>>
>> Nate
>>
>> -----Original Message-----
>> From: Adhemerval Zanella <adhemerval.zanella@linaro.org>
>> Sent: Tuesday, October 06, 2020 12:26
>> To: Karstens, Nate <Nate.Karstens@garmin.com>; mtk.manpages@gmail.com
>> Cc: linux-man@vger.kernel.org; libc-alpha@sourceware.org; 
>> willy@infradead.org
>> Subject: Re: [PATCH] system.3: Indicate MT-Unsafe
>>
>> CAUTION - EXTERNAL EMAIL: Do not click any links or open any attachments unless you trust the sender and know the content is safe.
>>
>>
>> On 06/10/2020 13:15, Nate Karstens via Libc-alpha wrote:
>>> The fact that system(3) does not support pthread_atfork(3) also 
>>> means that it is not thread safe. See the discussion for the 
>>> proposal of a close-on-fork flag in the 2020 April and May timeframe, especially:
>>>
>>> https://urldefense.com/v3/__https://lkml.org/lkml/2020/5/15/1067__;!
>>> !E 
>>> Jc4YC3iFmQ!D9YVAE760hT-YFoOT14KmIu4y2cjQb8ZflVgpX-3rxgBF2WvxyATUeQog
>>> ZF
>>> Ffv2sIQ$
>>>
>>> Signed-off-by: Nate Karstens <nate.karstens@garmin.com>
>>
>> Not sure if man pages characterizes file descriptor leak as mt-unsafe, at least we don't have this concept on glibc manual.  In fact, I think adding a MT-Unsafe mark to this potentially make any libc call that is not atomic potentially MT-Unsafe, either when they do not concurrent trigger race issues regarding memory semantic. At least I think it should add a 'race'
>> mark to indicate what exactly is MT-unsafe (as for other implementations).
>>
>>> ---
>>>  man3/system.3 | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/man3/system.3 b/man3/system.3 index 
>>> aef40417a..8730fabd3
>>> 100644
>>> --- a/man3/system.3
>>> +++ b/man3/system.3
>>> @@ -127,7 +127,7 @@ l l l.
>>>  Interface    Attribute       Value
>>>  T{
>>>  .BR system ()
>>> -T}   Thread safety   MT-Safe
>>> +T}   Thread safety   MT-Unsafe
>>>  .TE
>>>  .SH CONFORMING TO
>>>  POSIX.1-2001, POSIX.1-2008, C89, C99.
>>>
>>
>> ________________________________
>>
>> CONFIDENTIALITY NOTICE: This email and any attachments are for the sole use of the intended recipient(s) and contain information that may be Garmin confidential and/or Garmin legally privileged. If you have received this email in error, please notify the sender by reply email and delete the message. Any disclosure, copying, distribution or use of this communication (including attachments) by someone other than the intended recipient is prohibited. Thank you.
>>


--
Michael Kerrisk
Linux man-pages maintainer; https://urldefense.com/v3/__http://www.kernel.org/doc/man-pages/__;!!EJc4YC3iFmQ!FgWJQcfb_nvuQ86nksCLxQpy18-8nBQ5-8PypqUkGVoLBW9dHZQStNHhiy-HhtQLxw$
Linux/UNIX System Programming Training: https://urldefense.com/v3/__http://man7.org/training/__;!!EJc4YC3iFmQ!FgWJQcfb_nvuQ86nksCLxQpy18-8nBQ5-8PypqUkGVoLBW9dHZQStNHhiy-9TDzqeA$ 

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-10-30 20:59 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-06 16:15 [PATCH] system.3: Indicate MT-Unsafe Nate Karstens
2020-10-06 17:26 ` Adhemerval Zanella
2020-10-07 14:35   ` Karstens, Nate
2020-10-07 18:06     ` Adhemerval Zanella
2020-10-27  7:08       ` Michael Kerrisk (man-pages)
2020-10-30 20:59         ` Karstens, Nate

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).