Austin Group Defect Tracker

Aardvark Mark III


Viewing Issue Simple Details Jump to Notes ] Issue History ] Print ]
ID Category Severity Type Date Submitted Last Update
0001274 [1003.1(2016)/Issue7+TC2] Base Definitions and Headers Editorial Omission 2019-07-28 10:42 2019-08-23 16:15
Reporter dannyniu View Status public  
Assigned To
Priority normal Resolution Open  
Status New  
Name DannyNiu/NJF
Organization Individual
User Reference
Section <sys/types.h> header
Page Number 402-405
Line Number 13652-13746
Interp Status ---
Final Accepted Text
Summary 0001274: pid_t must fit in an int for definition of fcntl to be consistent.
Description As the return type of fcntl is int, and F_GETOWN returns the process (-group) receiving SIGIO and SIGURG signals, pid_t must fit into an int for the fcntl function to be consistent.

However, there's not yet any explicit mention of this requirement in the standard text.
Desired Action After line 13731, insert:

the width of pid_t shall be no greater than the width of int.
Tags No tags attached.
Attached Files

- Relationships

-  Notes
(0004496)
jilles (reporter)
2019-07-28 13:49

There is indeed an inconsistency here, but it could be resolved in various ways, depending on compatibility issues and risk of weirdnix implementations:

* Only require that actual process IDs fit in an int, without requiring anything new about the type pid_t.
* Require "the width of pid_t shall be no greater than the width of int".
* Require that the range of pid_t be equal to the range of int.
* Require that pid_t be a typedef for int.

Types smaller than int may have surprising behaviour due to integer promotion, which suggests not going out of our way to allow them.

In hindsight, it may have made more sense to define a new function instead of standardizing the existing fcntl() (compare tcgetpgrp()), but I don't think it is useful to add such a function now.
(0004497)
shware_systems (reporter)
2019-07-29 00:52

The requirement currently is "the width of pid_t shall be no greater than the width of intmax_t.", with the phrasing of <sys/types.h>, and limiting it to int is a potential breaking change. As a future consideration for implementations this phrasing is appropriate for <sys/types.h>, from a speed, packing, or alignment perspective, and should not be modified how this bug report requests, in my opinion.

The aspect of fcntl() that deals with this currently is it is required to set EOVERFLOW in errno if the process ID value is larger than INT_MAX or smaller than INT_MIN, per line 27934. From an 'are our bases covered?' perspective this is adequate. Fixing the interface so the possibility of this error is precluded requires invention, as in an F_GETPID variation that stores the result in an va_arg value cast as (pid_t *), not return it as cast to an int. The return value then could be a flag that the ID is valid or zero, or to check errno for other error conditions, such as fd not referring to a socket. Such a variation I do see as useful, and should be easy to implement.
(0004498)
geoffclare (manager)
2019-07-29 09:08

There is no problem with F_GETOWN here because the value to be returned is guaranteed to be able to fit in an int. This is because it is set via an int argument passed to fcntl() with F_SETOWN. (Okay, there might be an extension that could be used to set it to a larger value, but then there would be an equivalent extension to query it as well.)

So the only real problem I see is that it is not possible for a process with a PID greater than INT_MAX or a process group with a PGID greater than INT_MAX+1 to be set to receive SIGURG signals (without using an extension).

One solution would be to require that process IDs are always <= INT_MAX. Another would be to make F_SETOWN and F_GETOWN obsolescent and warn about the problem in APPLICATION USAGE.

There is certainly no need to alter the requirements about how pid_t can be defined.
(0004524)
eblake (manager)
2019-08-15 16:37

Linux fcntl has F_SETOWN_EX/F_GETOWN_EX that takes a pointer to:
                  struct f_owner_ex {
                      int type;
                      pid_t pid;
                  };
as a way that would allow access to pid_t larger than int (although at the moment pid_t on Linux is still int)
(0004525)
shware_systems (reporter)
2019-08-15 16:52

Re: 4524
Do you have a kernel.org URL to the header where that is typedef'd, or the man page showing that?
(0004526)
eblake (manager)
2019-08-15 16:58

<fcntl.h>, per http://man7.org/linux/man-pages/man2/fcntl.2.html [^]
(0004527)
shware_systems (reporter)
2019-08-15 19:03

According to the linux kernel header <repo URL>\include\uapi\asm-generic\fcntl.h, the definition is:
struct f_owner_ex {
    int type;
    __kernel_pid_t pid;
};

where __kernel_pid_t may not be assignment compatible with pid_t as defined by a <sys/types.h> that non-kernel sources have access to. There is code that establishes this compatibility when pid_t is expected to be typedef'd as int by <sys/types.h>, but it can be overridden by source files with a conflicting definition of __kernel_pid_t before the #include of that <fcntl.h> header.

This is a conflict between the docs and the source, at the least. Whether a bug report should be filed with the docs people or the source maintainers I couldn't say for sure. It appears the source allows the redefinition for backwards compatibility with processor specific optimizations for versions of the kernel before LINUX deferred to POSIX for some interface definitions. As such I lean to the docs being correct, the source has a regression.
(0004528)
eblake (manager)
2019-08-15 19:32
edited on: 2019-08-15 19:33

re: Note: 0004527
__kernel_pid_t is in the namespace reserved to the implementation, and the standard already forbids users from redefining pid_t (or any other type not starting with __ but ending with _t) from what the standard headers provide. Code that tries to redefine either type before including <fcntl.h> is broken for stomping on reserved namespace, and as such is irrelevant to proper usage of the interface. The libc (whether glibc or musl) is responsible for making sure the userspace struct in <fcntl.h> which defines struct f_owner_ex in terms of userspace pid_t will properly coordinate over to whatever syscall mechanism it uses to communicate with the kernel's internal struct, regardless of whether the kernel has different sizing internally from the public interface. In short, I see no bug here, in either the docs or the public headers.

(0004536)
geoffclare (manager)
2019-08-23 16:15

Proposed changes...

On page 238 line 8002 section <fcntl.h>, add:
The <fcntl.h> header shall define the f_owner_ex structure, which shall include at least the following members:
enum f_pid_type type     Discriminator for pid
pid_t pid                Process ID or process group ID

The <fcntl.h> header shall define the enumerated type enum f_pid_type whose enumerators shall include at least the following:

F_OWNER_PID
The pid member of f_owner_ex holds a process ID.
F_OWNER_PGRP
The pid member of f_owner_ex holds a process group ID.

On page 238 line 8016 section <fcntl.h>, change:
F_GETOWN
Get process or process group ID to receive SIGURG signals.
F_SETOWN
Set process or process group ID to receive SIGURG signals.
to:
F_GETOWN
Get process or process group ID to receive SIGURG signals, via int type.
F_GETOWN_EX
Get process or process group ID to receive SIGURG signals, via pid_t type.
F_SETOWN
Set process or process group ID to receive SIGURG signals, via int type.
F_SETOWN_EX
Set process or process group ID to receive SIGURG signals, via pid_t type.

On page 821 line 27817 section fnctl(), add:
F_GETOWN_EX
If fildes refers to a socket, get the process ID or process group ID specified to receive SIGURG signals when out-of-band data is available, by setting the type and pid members of the f_owner_ex structure pointed to by the third argument, arg. The value of type shall be F_OWNER_PID or F_OWNER_PGRP to indicate that pid contains a process ID or a process group ID, respectively. The value of pid shall be zero if no SIGURG signals are to be sent. If fildes does not refer to a socket, the results are unspecified.
F_SETOWN_EX
If fildes refers to a socket, set the process ID or process group ID specified to receive SIGURG signals when out-of-band data is available, using the value of the third argument, arg, taken as type pointer to struct f_owner_ex. The type and pid members of this structure shall be used as follows:
  • A pid value of zero shall indicate that no SIGURG signals are to be sent.
  • A type value of F_OWNER_PID and a positive pid value shall indicate that SIGURG signals are to be sent to the process ID specified in pid.
  • A type value of F_OWNER_PGRP and a positive pid value shall indicate that SIGURG signals are to be sent to the process group ID specified in pid.
If fildes does not refer to a socket, the results are unspecified.

Move the text from the F_SETOWN description on lines 27803-27816, beginning "Each time a SIGURG signal is sent" and ending "or by other means", to a separate paragraph after the F_SETOWN_EX description, and in it change:
Each time a SIGURG signal is sent
to:
For F_SETOWN and F_SETOWN_EX, each time a SIGURG signal is sent

On page 823 line 27923 section fcntl() (EINVAL shall fail), change:
The cmd argument is invalid, or the cmd argument is F_DUPFD or F_DUPFD_CLOEXEC and arg is negative or greater than or equal to {OPEN_MAX}, or the cmd argument is F_GETLK, F_SETLK, or F_SETLKW and the data pointed to by arg is not valid, or fildes refers to a file that does not support locking.
to:
The cmd argument is invalid; or the cmd argument is F_DUPFD or F_DUPFD_CLOEXEC and arg is negative or is greater than or equal to {OPEN_MAX}; or the cmd argument is F_SETOWN_EX and the type member of the f_owner_ex structure pointed to by arg is invalid, or the pid member is negative and the type member is F_OWNER_PID or F_OWNER_PGRP; or the cmd argument is F_GETLK, F_SETLK, or F_SETLKW and the data pointed to by arg is not valid, or fildes refers to a file that does not support locking.

On page 824 line 27938 section fcntl() (ESRCH), change:
F_SETOWN
to:
F_SETOWN or F_SETOWN_EX

On page 824 line 27944 section fcntl() (EINVAL may fail), change:
The cmd argument is F_SETOWN and the value of the argument is not valid as a process or process group identifier.
to:
The cmd argument is F_SETOWN and the value of arg is positive and is not valid as a process ID or the value of arg is negative and its absolute value is not valid as a process group ID; or the cmd argument is F_SETOWN_EX, the value of the type member of the f_owner_ex structure pointed to by arg is F_OWNER_PID, and the value of the pid member is not valid as a process ID; or the cmd argument is F_SETOWN_EX, the value of the type member of the f_owner_ex structure pointed to by arg is F_OWNER_PGRP, and the value of the pid member is not valid as a process group ID.

On page 824 line 27946 section fcntl() (EPERM), change:
F_SETOWN
to:
F_SETOWN or F_SETOWN_EX

On page 825 line 28011 section fcntl() APPLICATION USAGE, change:
On systems which do not perform permission checks at the time of an fcntl() call with F_SETOWN, ...
to:
On implementations where process IDs can be greater than INT_MAX, F_SETOWN cannot be used with process IDs greater than INT_MAX or process group IDs greater than INT_MAX+1 because the value is passed to fcntl() in an argument of type int. In this situation, F_SETOWN_EX should be used instead.

Similarly, if a process ID greater than INT_MAX or a process group ID greater than INT_MAX+1 has been set to receive SIGURG signals (using F_SETOWN_EX), F_GETOWN cannot be used to obtain the value because fcntl() returns the value as type int and will thus give an [EOVERFLOW] error for such values. F_GETOWN_EX should be used instead.

Note that the convention of negating a process group ID is only used with F_SETOWN and F_GETOWN; the pid member of the f_owner_ex structure used with F_SETOWN_EX and F_GETOWN_EX is not negated when it specifies a process group ID.

On systems which do not perform permission checks at the time of an fcntl() call with F_SETOWN or F_SETOWN_EX, ...

On page 827 line 28077 section fcntl() RATIONALE, add a new paragraph:
The F_SETOWN_EX and F_GETOWN_EX values for cmd and the associated f_owner_ex structure were adopted from the GNU C library. In addition to the values F_OWNER_PID and F_OWNER_PGRP for the type member, this also has F_OWNER_TID to specify that the pid member contains a thread ID. However, this relies on thread IDs being representable in a pid_t and so was not included in POSIX.1-20xx. The aim of adding F_SETOWN_EX and F_GETOWN_EX was to address the inability of F_SETOWN and F_GETOWN to handle process IDs greater than INT_MAX and process group IDs greater than INT_MAX+1, and this need is satisfied without including F_OWNER_TID.

- Issue History
Date Modified Username Field Change
2019-07-28 10:42 dannyniu New Issue
2019-07-28 10:42 dannyniu Name => DannyNiu/NJF
2019-07-28 10:42 dannyniu Organization => Individual
2019-07-28 10:42 dannyniu Section => <sys/types.h> header
2019-07-28 10:42 dannyniu Page Number => 402-405
2019-07-28 10:42 dannyniu Line Number => 13652-13746
2019-07-28 13:49 jilles Note Added: 0004496
2019-07-29 00:52 shware_systems Note Added: 0004497
2019-07-29 09:08 geoffclare Note Added: 0004498
2019-08-15 16:37 eblake Note Added: 0004524
2019-08-15 16:52 shware_systems Note Added: 0004525
2019-08-15 16:58 eblake Note Added: 0004526
2019-08-15 19:03 shware_systems Note Added: 0004527
2019-08-15 19:32 eblake Note Added: 0004528
2019-08-15 19:33 eblake Note Edited: 0004528
2019-08-23 16:15 geoffclare Note Added: 0004536


Mantis 1.1.6[^]
Copyright © 2000 - 2008 Mantis Group
Powered by Mantis Bugtracker