[PATCH] semctl.2: Correct SEM_STAT_ANY description

[PATCH] semctl.2: Correct SEM_STAT_ANY description

[Yang Xu]

Since kernel commit a280d6dc77eb ("ipc/sem: introduce semctl(SEM_STAT_ANY)"),
it only skips read access check when using SEM_STAT_ANY command. And it should
use the semid_ds struct instead of seminfo struct. Fix this.

Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
---
 man2/semctl.2 | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/man2/semctl.2 b/man2/semctl.2
index dd3fb077b..a7462c5cc 100644
--- a/man2/semctl.2
+++ b/man2/semctl.2
@@ -297,8 +297,8 @@ all semaphore sets on the system.
 .TP
 .BR SEM_STAT_ANY " (Linux-specific, since Linux 4.17)"
 Return a
-.I seminfo
-structure containing the same information as for
+.I semid_ds
+structure as for
 .BR SEM_STAT .
 However,
 .I sem_perm.mode
-- 
2.23.0

Re: [PATCH] semctl.2: Correct SEM_STAT_ANY description

[Alejandro Colomar (mailing lists; readonly)]

Hi Yang,

It looks good to me.
I'll add a few people that might want to comment.

Thanks,

Alex

On 12/22/20 6:55 AM, Yang Xu wrote:
> Since kernel commit a280d6dc77eb ("ipc/sem: introduce semctl(SEM_STAT_ANY)"),
> it only skips read access check when using SEM_STAT_ANY command. And it should
> use the semid_ds struct instead of seminfo struct. Fix this.
> 
> Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
> ---
>  man2/semctl.2 | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/man2/semctl.2 b/man2/semctl.2
> index dd3fb077b..a7462c5cc 100644
> --- a/man2/semctl.2
> +++ b/man2/semctl.2
> @@ -297,8 +297,8 @@ all semaphore sets on the system.
>  .TP
>  .BR SEM_STAT_ANY " (Linux-specific, since Linux 4.17)"
>  Return a
> -.I seminfo
> -structure containing the same information as for
> +.I semid_ds
> +structure as for
>  .BR SEM_STAT .
>  However,
>  .I sem_perm.mode
> 

Re: [PATCH] semctl.2: Correct SEM_STAT_ANY description

[Yang Xu]

Hi Alex
I think it is a smple fix. So ping!

Best Regards
Yang Xu
> Hi Yang,
>
> It looks good to me.
> I'll add a few people that might want to comment.
>
> Thanks,
>
> Alex
>
> On 12/22/20 6:55 AM, Yang Xu wrote:
>> Since kernel commit a280d6dc77eb ("ipc/sem: introduce semctl(SEM_STAT_ANY)"),
>> it only skips read access check when using SEM_STAT_ANY command. And it should
>> use the semid_ds struct instead of seminfo struct. Fix this.
>>
>> Signed-off-by: Yang Xu<xuyang2018.jy@cn.fujitsu.com>
>> ---
>>   man2/semctl.2 | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/man2/semctl.2 b/man2/semctl.2
>> index dd3fb077b..a7462c5cc 100644
>> --- a/man2/semctl.2
>> +++ b/man2/semctl.2
>> @@ -297,8 +297,8 @@ all semaphore sets on the system.
>>   .TP
>>   .BR SEM_STAT_ANY " (Linux-specific, since Linux 4.17)"
>>   Return a
>> -.I seminfo
>> -structure containing the same information as for
>> +.I semid_ds
>> +structure as for
>>   .BR SEM_STAT .
>>   However,
>>   .I sem_perm.mode
>>
>
>
> .
>

Re: [PATCH] semctl.2: Correct SEM_STAT_ANY description

[Manfred Spraul]

Hi,


On 12/22/20 12:55 PM, Alejandro Colomar (mailing lists; readonly) wrote:
> Hi Yang,
>
> It looks good to me.
> I'll add a few people that might want to comment.

The code returns a semid_ds structure, and if I take strace as reference implementation, then user space expects a semid_ds as well.
https://github.com/strace/strace/commit/8f0870a780bfd8cd9a91c3b7ad05baccda10bc84

In addition, the current page is self-inconsistent: seminfo doesn't contain sem_perm.

The pages for msgctl and shmctl are correct, i.e. no further obvious inconsistencies.

Thus: The man page for semctl is incorrect, the page needs to be updated.

Acked-by: manfred@colorfullife.com

> Thanks,
>
> Alex
>
> On 12/22/20 6:55 AM, Yang Xu wrote:
>> Since kernel commit a280d6dc77eb ("ipc/sem: introduce semctl(SEM_STAT_ANY)"),
>> it only skips read access check when using SEM_STAT_ANY command. And it should
>> use the semid_ds struct instead of seminfo struct. Fix this.
>>
>> Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
>> ---
>>   man2/semctl.2 | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/man2/semctl.2 b/man2/semctl.2
>> index dd3fb077b..a7462c5cc 100644
>> --- a/man2/semctl.2
>> +++ b/man2/semctl.2
>> @@ -297,8 +297,8 @@ all semaphore sets on the system.
>>   .TP
>>   .BR SEM_STAT_ANY " (Linux-specific, since Linux 4.17)"
>>   Return a
>> -.I seminfo
>> -structure containing the same information as for
>> +.I semid_ds
>> +structure as for
>>   .BR SEM_STAT .
>>   However,
>>   .I sem_perm.mode
>>

Re: [PATCH] semctl.2: Correct SEM_STAT_ANY description

[Alejandro Colomar (mailing lists; readonly)]

On 12/29/20 11:38 AM, Manfred Spraul wrote:
> Hi,
> 
> 
> On 12/22/20 12:55 PM, Alejandro Colomar (mailing lists; readonly) wrote:
>> Hi Yang,
>>
>> It looks good to me.
>> I'll add a few people that might want to comment.
> 
> The code returns a semid_ds structure, and if I take strace as reference
> implementation, then user space expects a semid_ds as well.
> https://github.com/strace/strace/commit/8f0870a780bfd8cd9a91c3b7ad05baccda10bc84
> 
> 
> In addition, the current page is self-inconsistent: seminfo doesn't
> contain sem_perm.
> 
> The pages for msgctl and shmctl are correct, i.e. no further obvious
> inconsistencies.
> 
> Thus: The man page for semctl is incorrect, the page needs to be updated.
> 
> Acked-by: manfred@colorfullife.com
> 
>> Thanks,
>>
>> Alex
>>
>> On 12/22/20 6:55 AM, Yang Xu wrote:
>>> Since kernel commit a280d6dc77eb ("ipc/sem: introduce
>>> semctl(SEM_STAT_ANY)"),
>>> it only skips read access check when using SEM_STAT_ANY command. And
>>> it should
>>> use the semid_ds struct instead of seminfo struct. Fix this.
>>>
>>> Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>

Hi Yang Xu and Manfred,

Thanks for the ping and for the acked-by!

Patch applied.

Thanks,

Alex

>>> ---
>>>   man2/semctl.2 | 4 ++--
>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/man2/semctl.2 b/man2/semctl.2
>>> index dd3fb077b..a7462c5cc 100644
>>> --- a/man2/semctl.2
>>> +++ b/man2/semctl.2
>>> @@ -297,8 +297,8 @@ all semaphore sets on the system.
>>>   .TP
>>>   .BR SEM_STAT_ANY " (Linux-specific, since Linux 4.17)"
>>>   Return a
>>> -.I seminfo
>>> -structure containing the same information as for
>>> +.I semid_ds
>>> +structure as for
>>>   .BR SEM_STAT .
>>>   However,
>>>   .I sem_perm.mode
>>>
> 

Re: [PATCH] semctl.2: Correct SEM_STAT_ANY description

[Yang Xu]

Hi Manfred
> Hi,
>
>
> On 12/22/20 12:55 PM, Alejandro Colomar (mailing lists; readonly) wrote:
>> Hi Yang,
>>
>> It looks good to me.
>> I'll add a few people that might want to comment.
>
> The code returns a semid_ds structure, and if I take strace as reference
> implementation, then user space expects a semid_ds as well.
> https://github.com/strace/strace/commit/8f0870a780bfd8cd9a91c3b7ad05baccda10bc84
>
>
> In addition, the current page is self-inconsistent: seminfo doesn't
> contain sem_perm.
semctl manpage doesn't say seminfo contain sem_perm. Or, I miss something?
>
> The pages for msgctl and shmctl are correct, i.e. no further obvious
> inconsistencies.
>
> Thus: The man page for semctl is incorrect, the page needs to be updated.
>
> Acked-by: manfred@colorfullife.com
>
>> Thanks,
>>
>> Alex
>>
>> On 12/22/20 6:55 AM, Yang Xu wrote:
>>> Since kernel commit a280d6dc77eb ("ipc/sem: introduce
>>> semctl(SEM_STAT_ANY)"),
>>> it only skips read access check when using SEM_STAT_ANY command. And
>>> it should
>>> use the semid_ds struct instead of seminfo struct. Fix this.
>>>
>>> Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
>>> ---
>>> man2/semctl.2 | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/man2/semctl.2 b/man2/semctl.2
>>> index dd3fb077b..a7462c5cc 100644
>>> --- a/man2/semctl.2
>>> +++ b/man2/semctl.2
>>> @@ -297,8 +297,8 @@ all semaphore sets on the system.
>>> .TP
>>> .BR SEM_STAT_ANY " (Linux-specific, since Linux 4.17)"
>>> Return a
>>> -.I seminfo
>>> -structure containing the same information as for
>>> +.I semid_ds
>>> +structure as for
>>> .BR SEM_STAT .
>>> However,
>>> .I sem_perm.mode
>>>
>
>
>
> .
>

Re: [PATCH] semctl.2: Correct SEM_STAT_ANY description

[Manfred Spraul]

On 12/30/20 3:03 AM, Yang Xu wrote:
> Hi Manfred
>> Hi,
>>
>>
>> On 12/22/20 12:55 PM, Alejandro Colomar (mailing lists; readonly) wrote:
>>> Hi Yang,
>>>
>>> It looks good to me.
>>> I'll add a few people that might want to comment.
>>
>> The code returns a semid_ds structure, and if I take strace as reference
>> implementation, then user space expects a semid_ds as well.
>> https://github.com/strace/strace/commit/8f0870a780bfd8cd9a91c3b7ad05baccda10bc84 
>>
>>
>>
>> In addition, the current page is self-inconsistent: seminfo doesn't
>> contain sem_perm.
> semctl manpage doesn't say seminfo contain sem_perm. Or, I miss something?

The current man page says that SEM_STAT_ANY returns a seminfo structure, 
without checking sem_perm.

This is self-inconsistent: struct seminfo contains global 
(per-namespace) information, sem_perm.mode is a per-array information.

I.e.: It is clear that the man page needs to be updated, and not the code.

> $rpm -qf /usr/share/man/man2/semctl.2.gz
> $ man-pages-5.07-3.fc33.noarch
>        SEM_STAT_ANY (Linux-specific, since Linux 4.17)
>               Return a seminfo structure containing the same 
> information as for SEM_STAT.  However, sem_perm.mode is not checked 
> for read access for semid meaning
>               that any user can employ this operation (just as any 
> user may read /proc/sysvipc/sem to obtain the same information).
>
>

Re: [PATCH] semctl.2: Correct SEM_STAT_ANY description

[Michael Kerrisk (man-pages)]

Hi Manfred,

On 12/30/20 12:20 PM, Manfred Spraul wrote:
> On 12/30/20 3:03 AM, Yang Xu wrote:
>> Hi Manfred
>>> Hi,
>>>
>>>
>>> On 12/22/20 12:55 PM, Alejandro Colomar (mailing lists; readonly) wrote:
>>>> Hi Yang,
>>>>
>>>> It looks good to me.
>>>> I'll add a few people that might want to comment.
>>>
>>> The code returns a semid_ds structure, and if I take strace as reference
>>> implementation, then user space expects a semid_ds as well.
>>> https://github.com/strace/strace/commit/8f0870a780bfd8cd9a91c3b7ad05baccda10bc84 
>>>
>>>
>>>
>>> In addition, the current page is self-inconsistent: seminfo doesn't
>>> contain sem_perm.
>> semctl manpage doesn't say seminfo contain sem_perm. Or, I miss something?
> 
> The current man page says that SEM_STAT_ANY returns a seminfo structure, 
> without checking sem_perm.
> 
> This is self-inconsistent: struct seminfo contains global 
> (per-namespace) information, sem_perm.mode is a per-array information.
> 
> I.e.: It is clear that the man page needs to be updated, and not the code.

After reading this thread, I'm not quite clear. Do you mean some 
additional change is required after Xang Yu's patch?

Thanks,

Michael

>> $rpm -qf /usr/share/man/man2/semctl.2.gz
>> $ man-pages-5.07-3.fc33.noarch
>>        SEM_STAT_ANY (Linux-specific, since Linux 4.17)
>>               Return a seminfo structure containing the same 
>> information as for SEM_STAT.  However, sem_perm.mode is not checked 
>> for read access for semid meaning
>>               that any user can employ this operation (just as any 
>> user may read /proc/sysvipc/sem to obtain the same information).
>>
>>
> 


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

Re: [PATCH] semctl.2: Correct SEM_STAT_ANY description

[Manfred Spraul]

Hi Michael,

On 12/30/20 2:35 PM, Michael Kerrisk (man-pages) wrote:
> Hi Manfred,
>
> On 12/30/20 12:20 PM, Manfred Spraul wrote:
>> On 12/30/20 3:03 AM, Yang Xu wrote:
>>> Hi Manfred
>>>> Hi,
>>>>
>>>>
>>>> On 12/22/20 12:55 PM, Alejandro Colomar (mailing lists; readonly) wrote:
>>>>> Hi Yang,
>>>>>
>>>>> It looks good to me.
>>>>> I'll add a few people that might want to comment.
>>>> The code returns a semid_ds structure, and if I take strace as reference
>>>> implementation, then user space expects a semid_ds as well.
>>>> https://github.com/strace/strace/commit/8f0870a780bfd8cd9a91c3b7ad05baccda10bc84
>>>>
>>>>
>>>>
>>>> In addition, the current page is self-inconsistent: seminfo doesn't
>>>> contain sem_perm.
>>> semctl manpage doesn't say seminfo contain sem_perm. Or, I miss something?
>> The current man page says that SEM_STAT_ANY returns a seminfo structure,
>> without checking sem_perm.
>>
>> This is self-inconsistent: struct seminfo contains global
>> (per-namespace) information, sem_perm.mode is a per-array information.
>>
>> I.e.: It is clear that the man page needs to be updated, and not the code.
> After reading this thread, I'm not quite clear. Do you mean some
> additional change is required after Xang Yu's patch?

Sorry for the confusion:

No further changes are required.

(I have crosschecked shmctl, msgctl, actual code, and strace as user 
space example: After Xang Yu's patch is applied, everything is consistent)

--

     Manfred

Re: [PATCH] semctl.2: Correct SEM_STAT_ANY description

[Michael Kerrisk (man-pages)]

On 12/30/20 3:33 PM, Manfred Spraul wrote:
> Hi Michael,
> 
> On 12/30/20 2:35 PM, Michael Kerrisk (man-pages) wrote:
>> Hi Manfred,
>>
>> On 12/30/20 12:20 PM, Manfred Spraul wrote:
>>> On 12/30/20 3:03 AM, Yang Xu wrote:
>>>> Hi Manfred
>>>>> Hi,
>>>>>
>>>>>
>>>>> On 12/22/20 12:55 PM, Alejandro Colomar (mailing lists; readonly) wrote:
>>>>>> Hi Yang,
>>>>>>
>>>>>> It looks good to me.
>>>>>> I'll add a few people that might want to comment.
>>>>> The code returns a semid_ds structure, and if I take strace as reference
>>>>> implementation, then user space expects a semid_ds as well.
>>>>> https://github.com/strace/strace/commit/8f0870a780bfd8cd9a91c3b7ad05baccda10bc84
>>>>>
>>>>>
>>>>>
>>>>> In addition, the current page is self-inconsistent: seminfo doesn't
>>>>> contain sem_perm.
>>>> semctl manpage doesn't say seminfo contain sem_perm. Or, I miss something?
>>> The current man page says that SEM_STAT_ANY returns a seminfo structure,
>>> without checking sem_perm.
>>>
>>> This is self-inconsistent: struct seminfo contains global
>>> (per-namespace) information, sem_perm.mode is a per-array information.
>>>
>>> I.e.: It is clear that the man page needs to be updated, and not the code.
>> After reading this thread, I'm not quite clear. Do you mean some
>> additional change is required after Xang Yu's patch?
> 
> Sorry for the confusion:
> 
> No further changes are required.
> 
> (I have crosschecked shmctl, msgctl, actual code, and strace as user 
> space example: After Xang Yu's patch is applied, everything is consistent)

Thanks, Manfred! It's clear to me now.

Cheers,

Michael


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

Re: [PATCH] semctl.2: Correct SEM_STAT_ANY description

[Yang Xu]

Hi Michael

I think now it is nothing block this patch.

Best Regards
Yang Xu
> On 12/30/20 3:33 PM, Manfred Spraul wrote:
>> Hi Michael,
>>
>> On 12/30/20 2:35 PM, Michael Kerrisk (man-pages) wrote:
>>> Hi Manfred,
>>>
>>> On 12/30/20 12:20 PM, Manfred Spraul wrote:
>>>> On 12/30/20 3:03 AM, Yang Xu wrote:
>>>>> Hi Manfred
>>>>>> Hi,
>>>>>>
>>>>>>
>>>>>> On 12/22/20 12:55 PM, Alejandro Colomar (mailing lists; readonly) wrote:
>>>>>>> Hi Yang,
>>>>>>>
>>>>>>> It looks good to me.
>>>>>>> I'll add a few people that might want to comment.
>>>>>> The code returns a semid_ds structure, and if I take strace as reference
>>>>>> implementation, then user space expects a semid_ds as well.
>>>>>> https://github.com/strace/strace/commit/8f0870a780bfd8cd9a91c3b7ad05baccda10bc84
>>>>>>
>>>>>>
>>>>>>
>>>>>> In addition, the current page is self-inconsistent: seminfo doesn't
>>>>>> contain sem_perm.
>>>>> semctl manpage doesn't say seminfo contain sem_perm. Or, I miss something?
>>>> The current man page says that SEM_STAT_ANY returns a seminfo structure,
>>>> without checking sem_perm.
>>>>
>>>> This is self-inconsistent: struct seminfo contains global
>>>> (per-namespace) information, sem_perm.mode is a per-array information.
>>>>
>>>> I.e.: It is clear that the man page needs to be updated, and not the code.
>>> After reading this thread, I'm not quite clear. Do you mean some
>>> additional change is required after Xang Yu's patch?
>>
>> Sorry for the confusion:
>>
>> No further changes are required.
>>
>> (I have crosschecked shmctl, msgctl, actual code, and strace as user
>> space example: After Xang Yu's patch is applied, everything is consistent)
>
> Thanks, Manfred! It's clear to me now.
>
> Cheers,
>
> Michael
>
>

[PATCH] semctl.2: Correct SEM_STAT_ANY description

[Alejandro Colomar]

From: Yang Xu <xuyang2018.jy@cn.fujitsu.com>

Since kernel commit a280d6dc77eb
("ipc/sem: introduce semctl(SEM_STAT_ANY)"),
it only skips read access check when using SEM_STAT_ANY command.
And it should use the semid_ds struct instead of seminfo struct.
Fix this.

Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
Acked-by: Manfred Spraul <manfred@colorfullife.com>
Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
---

Hi Michael,

Here's a recent patch from Yang Xu.

Cheers,

Alex

 man2/semctl.2 | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/man2/semctl.2 b/man2/semctl.2
index ed5e79efd..e562d0bc4 100644
--- a/man2/semctl.2
+++ b/man2/semctl.2
@@ -297,8 +297,8 @@ all semaphore sets on the system.
 .TP
 .BR SEM_STAT_ANY " (Linux-specific, since Linux 4.17)"
 Return a
-.I seminfo
-structure containing the same information as for
+.I semid_ds
+structure as for
 .BR SEM_STAT .
 However,
 .I sem_perm.mode

base-commit: c55f66855eccfcd92b35fe7b13a326121f2ee0fd
-- 
2.29.2

Re: [PATCH] semctl.2: Correct SEM_STAT_ANY description

[Michael Kerrisk (man-pages)]

Hi Alex, hi Xang,

On 12/29/20 12:03 PM, Alejandro Colomar wrote:
> From: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
> 
> Since kernel commit a280d6dc77eb
> ("ipc/sem: introduce semctl(SEM_STAT_ANY)"),
> it only skips read access check when using SEM_STAT_ANY command.
> And it should use the semid_ds struct instead of seminfo struct.
> Fix this.
> 
> Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
> Acked-by: Manfred Spraul <manfred@colorfullife.com>
> Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>

Patch applied. My apologies for the delay.

Thanks,

Michael

> ---
> 
> Hi Michael,
> 
> Here's a recent patch from Yang Xu.
> 
> Cheers,
> 
> Alex
> 
>  man2/semctl.2 | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/man2/semctl.2 b/man2/semctl.2
> index ed5e79efd..e562d0bc4 100644
> --- a/man2/semctl.2
> +++ b/man2/semctl.2
> @@ -297,8 +297,8 @@ all semaphore sets on the system.
>  .TP
>  .BR SEM_STAT_ANY " (Linux-specific, since Linux 4.17)"
>  Return a
> -.I seminfo
> -structure containing the same information as for
> +.I semid_ds
> +structure as for
>  .BR SEM_STAT .
>  However,
>  .I sem_perm.mode
> 
> base-commit: c55f66855eccfcd92b35fe7b13a326121f2ee0fd
> 


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