Anonymous | Login | 2023-12-07 11:23 UTC |
Main | My View | View Issues | Change Log | Docs |
Viewing Issue Simple Details [ Jump to Notes ] | [ Issue History ] [ Print ] | ||||||
ID | Category | Severity | Type | Date Submitted | Last Update | ||
0001283 | [1003.1(2016/18)/Issue7+TC2] System Interfaces | Objection | Clarification Requested | 2019-09-04 09:16 | 2019-12-05 11:20 | ||
Reporter | geoffclare | View Status | public | ||||
Assigned To | |||||||
Priority | normal | Resolution | Accepted As Marked | ||||
Status | Applied | ||||||
Name | Geoff Clare | ||||||
Organization | The Open Group | ||||||
User Reference | |||||||
Section | chmod() | ||||||
Page Number | 665 | ||||||
Line Number | 22780 | ||||||
Interp Status | Approved | ||||||
Final Accepted Text | See Note: 0004592. | ||||||
Summary | 0001283: should chmod() ignore file type bits in st_mode | ||||||
Description |
The description of chmod() says:The chmod() function shall change S_ISUID, S_ISGID, [XSI]S_ISVTX,[/XSI] and the file permission bits of the file named by the pathname pointed to by the path argument to the corresponding bits in the mode argument. The way this is worded implies that only the specified bits in the mode argument are examined by chmod(), and it should ignore other bits. However, there is also a "may fail" EINVAL error: The value of the mode argument is invalid. whose presence could be seen as casting doubt on this interpretation. Alternatively, perhaps this EINVAL error is there so that non-XSI implementations that don't support S_ISVTX can fail the chmod() if an application attempts to set that bit. Another consideration is whether any implementations support additional settable bits as an extension. If such extensions are allowed, the question becomes whether chmod() should ignore the bits used to encode the file type (and any other "read-only" bits that can be set in st_mode). This affects code which modifies file permissions by obtaining the st_mode value for a file and setting or clearing some permission bits in the value before passing it to chmod(). For example: stat(file, &sbuf); chmod(file, sbuf.st_mode | S_IRWXU);If chmod() is supposed to ignore file-type (and other read-only) st_mode bits then this is safe, since any other bits outside 07777 that are set in st_mode would correspond to additional settable bits supported as an extension. I have some code which does something like this and it has worked fine for many years on many systems. However, recently a system failed the chmod() with EINVAL. Is this code supposed to be portable, or is it relying on implementations choosing not to fail with EINVAL if read-only st_mode bits are set the requested mode? |
||||||
Desired Action |
OPTION 1 On page 665 line 22779 section chmod(), change: The chmod() function shall change S_ISUID, S_ISGID, [XSI]S_ISVTX,[/XSI] and the file permission bits of the file named by the pathname pointed to by the path argument to the corresponding bits in the mode argument. The application shall ensure that the effective user ID of the process matches the owner of the file or the process has appropriate privileges in order to do this.to: The chmod() function shall change S_ISUID, S_ISGID, [XSI]S_ISVTX,[/XSI] and the file permission bits of the file named by the pathname pointed to by the path argument to the corresponding bits in the mode argument. If any bits that can be set in the st_mode value returned by stat() but cannot be changed using chmod(), such as the bits that are used to encode the file type, are set in the mode argument, these read-only st_mode bits shall be ignored. On page 666 line 22834 section chmod(), change: The value of the mode argument is invalid.to: The value of the mode argument, ignoring read-only st_mode bits (see the DESCRIPTION), is invalid. On page 667 line 22866 section chmod() add a new example: Modifying File Permissions OPTION 2 On page 665 line 22779 section chmod(), change: The chmod() function shall change S_ISUID, S_ISGID, [XSI]S_ISVTX,[/XSI] and the file permission bits of the file named by the pathname pointed to by the path argument to the corresponding bits in the mode argument. The application shall ensure that the effective user ID of the process matches the owner of the file or the process has appropriate privileges in order to do this.to: The chmod() function shall change S_ISUID, S_ISGID, [XSI]S_ISVTX,[/XSI] and the file permission bits of the file named by the pathname pointed to by the path argument to the corresponding bits in the mode argument. If any other bits are set in the mode argument, the chmod() function may treat the mode argument value as invalid; if the implementation does not support the XSI option, setting the 01000 bit may also cause the mode value to be treated as invalid (01000 is the value of S_ISVTX on implementations that support the XSI option). On page 667 line 22866 section chmod() add a new example: Modifying File Permissions On page 667 line 22878 section chmod() add a new first paragraph to APPLICATION USAGE: When adding or removing permission bits in a file's mode, the st_mode value obtained from, for example, stat() should be masked with 07777 in order to ensure an [EINVAL] error does not occur. |
||||||
Tags | tc3-2008 | ||||||
Attached Files | |||||||
|
![]() |
|
(0004547) shware_systems (reporter) 2019-09-04 17:10 |
This was discussed years ago, due to confusion I had about encoding device types in st_mode. Having additional bits in st_mode is not a conforming extension, to keep it compatible with type short, though it's arguable the ISVTX bit may have a different label and function on non-XSI systems. The means of extending stat left open is to define a new field, not add bits. Then set/test macros that take the entire struct as argument and not the mode field alone, are to be defined, similar to the test for being TYM or SHM, to hide non-portable naming. I believe the reason for EINVAL being may fail is some device types may not support the particular function a bit designates; for example, a read only FIFO may consider it invalid to attempt to set any W or X permission bits. |
(0004548) kre (reporter) 2019-09-04 22:40 |
Re bugnote: 4547 There is nothing in posix (nor in some implementations) that suggests that a mode_t should be 16 bits (short). Adding bits is entirely possible (and needed to define new file types, if not so much for more permissions, which would be hard to do in a portable manner). Re the proposed resolution: I don't much like using 07777 as a "magic" number, even though the values of all the relevant bits are defined, and fit (exactly) into that value. Nor am I convinced that giving in to whatever implementation returned EINVAL is necessarily the corrcet thing to do. What would seem more reasonable to me would be to require implementations accept either all 0's in the "other" bits, or a value that exactly matches the current settings of those bits, and is permitted to return EINVAL only in cases where the application has attempted to use chmod to alter the state of those bits (if it isn't already required somewhere, I'd also add, somewhere, not related to chmod, a requirement that it is invalid to have all the other bits 0 for any existing file, whatever its type). EINVAL in cases where an attempt is made to set permissions that are impossible for the file in question is OK as well of course. |
(0004549) kre (reporter) 2019-09-04 23:03 |
OPTION 3: On page 665 line 22779 section chmod(), change: [the same words as in options 1 & 2] to The chmod() function shall change S_ISUID, S_ISGID, [XSI]S_ISVTX,[/XSI] and the file permission bits of the file named by the pathname pointed to by the path argument to the corresponding bits in the mode argument. Any other bits set in the mode argument that differ from the value that would be returned in st_mode value returned by stat() of the same path, may be treated as an error. If the effective user ID of the process does not match the owner of the file and the process does not have appropriate privileges, the chmod() function shall fail. On page 666 line 22834 section chmod(), change nothing. Add the same new example on page 667 line 22866 as is shown for OPTION 1. |
(0004550) shware_systems (reporter) 2019-09-05 01:48 |
Re: 4548 It is not that it has to be a short, just that what is designated for the type can fit in it and no more file types or mode bits will be added to break that possibility. I was thinking similarly to you, actually, that the intent was it might be extended, with each new file type being represented by its own bit (so testing for or excluding multiple types in one mask/compare was plausible) but this is not the case. |
(0004551) kre (reporter) 2019-09-05 03:46 |
st_mode in original unix (pdp-11) was an int (16 bits). When ported to 32 bit systems a choice needed to be made .. keep it an int (now 32 bits) preserving source code compat, or make it short (u_short) keeping binary compat with the older version. Either choice would have been reasonable, and it is possible both were made by different implementations. BSD systems have mode_t as uint32_t (I think all of them, but did not check.) Representing file types as individual bits was never going to happen, that ship sailed in the earliest implementations, but adding more new file types is certainly still possible (many more have been added to the original set over the years) and any implementation is free to add as many as it likes. With only 4 bits left available in a short, there's only room for 15 different file types (reserving 0 as unavailable), but with wider mode_t many more can be handled - not that I know of any systems with more than 15. An implementation, using interfaces that are POSIX extensions could also place other info in the st_mode field - as long as the access methods defined by the standard still all work, I see no issue with that. None of that is relevant to chmod() though, which only ever affects those bottom 12 bits. I think we should keep the standard such that giving a mode arg to chmod that is < (1<<12) and which contains nothing inappropriate for the actual file type, should always be valid (I don't believe there's any question about that one), but I also believe it should be OK to use the value obtained from st_mode after a stat() while changing any of those bottom 12 bits. That would be option 1, except I also think it reasonable to allow an implementation to reject the mode (EINVAL) if any of the other bits from the st_mode value were altered (so, the "ignored" in option 1 is not my choice). Note that is "allowed" to the implementation, not required of it. If an implementation decides to simply ignore all but the bottom 12 bits that is OK too. |
(0004552) kre (reporter) 2019-09-05 04:04 |
Just in case it is not clear, I know that the standard is not going to require more that 16 bit mode_t and that new file types defined by POSIX will be defined in a way that makes the implementation more flexible ... but for chmod() purposes, that is irrelevant - what matters is that implementations with a wider mode_t can choose to add new file types there - both standard and non-standard ones. Code that manipulates the st_mode field returned from stat() needs to be aware of that. That includes code planning on doing chmod(). There are 2 choices for such code, either pass only the bottom 12 bits of the mode_t value as the mode arg to chmod(), or pass the entire st_mode changing only some of the bottom 12 bits. Either of those 2 should be specified to work. Any other manipulation must be permitted to fail (but not required to fail). |
(0004553) geoffclare (manager) 2019-09-05 08:47 edited on: 2019-09-05 09:01 |
Re: Note: 0004549 I can see a few of problems with option 3: * It can't just talk about the st_mode that would be returned by stat(); it needs to relate it to stat() for chmod() and for fchmodat() with flag 0, and to lstat() for fchmodat() with flag AT_SYMLINK_NOFOLLOW. (I realise now that option 1 also has a similar problem, but it can be fixed there just by changing stat() to lstat() in "can be set ... by stat()".) * If an application clears one or more bits in the part of st_mode that encodes the file type while leaving at least one of those bits set, I think your intention is that this is allowed to be an error, but it's not clear from the wording. Maybe this would be clearer: If the value represented by all of the other bits in the mode argument is non-zero and differs from the value that those bits would represent in the st_mode value returned by ... of the same path, this may be treated as an error. * There's no point changing the DESCRIPTION to narrow down the set of mode values that can be treated as an error if the EINVAL wording is left as-is, because it currently gives implementations carte blanche. |
(0004592) Don Cragun (manager) 2019-10-03 16:04 edited on: 2019-10-03 16:21 |
The standard is unclear on this issue, and no conformance distinction can be made between alternative implementations based on this. This is being referred to the sponsor. Rationale: ------------- The selected option matches current existing practice, but alternative implementations have been investigated. Notes to the Editor (not part of this interpretation): ------------------------------------------------------- Implement OPTION 1 from the Desired Action, but change: the st_mode value returned by stat() but cannotto: the st_mode value returned by lstat() or stat() but cannot |
(0004593) shware_systems (reporter) 2019-10-03 16:28 |
Re: 4551 Per the phone discussion 2019-10-03, there is no S_IFMT value reserved as unavailable, so a total of 16 types could be encoded in it and still fit in a short. Per the Future Directions section any use by an implementation of other values, including a reservation of 0 for any purpose, in S_IFMT are non-portable. To show a stat structure does not represent any actual file, an implementation would need to define an S_TYPEISxxx macro for testing that state; an S_IFMT value of 0 does not serve this purpose. |
(0004594) eblake (manager) 2019-10-03 16:30 |
Option 3 is less like existing practice. The following example on Linux is proof that setting S_IFLNK bits in st.st_mode before calling chmod() on S_IFREG is ignored, not rejected: $ cat file.c #include <stdio.h> #include <errno.h> #include <sys/stat.h> int main(void) { struct stat st; lstat("b", &st); int i = chmod("a", st.st_mode); printf ("%d %d\n", i, errno); } $ touch a $ ln -s a b $ ./a.out 0 0 |
(0004595) kre (reporter) 2019-10-04 21:57 |
Re Note: 0004593 I have no idea why the phone call even wasted time discussing that, as it is completely off topic for the issue at hand, but ... I agree, POSIX does not reserve a value for *nothing* as a file type - why would it? By definition such a thing does not exist, and so cannot be returned by any of the stat() functions, and would be just one of many (presumably) invalid values to hand to mknod() if posix even defined that interface, so calling that one out there would make no sense either. That said, I have never seen an implementation where (st_mode & S_IFMT) == 0 represented anything, and I cannot imagine a situation where an implementation would ever do that ... at the very least, that would be the last S_IFMT value assigned, and whenever values are being assigned from a potentially unbounded set (ie: where we cannot prove we will never need any more) and you're reaching the end of the previously allocated space, some method needs to be found to add new values in another way. Since the implementation knows it is going to need to do that, and probably soon(ish) it is far more reasonable to simply do it now (whenever that "now" is) rather that assigning 0 and deferring the work until later. POSIX has recognised that, and made it clear that whatever other way that implementations choose will be OK (the tests for file types for new file types will never be expected to work with only st_mode as the arg, but only with the whole structure available), so why would an implementation use that one, never before used value, even though it might poausibly have been use as the stating point, for regular files - but wasn't - for anything? But as I said, all of this is off topic for chmod, which explicitly is not touching any of those bits, and completely irrelevant to anything here. Re Note: 0004594 .. I agree, that is the common implementation, and I certainly would not have disallowed that. I thought it was clear, that my only point was to allow an implementation to give an error in that case if it wants to. That is, I see no particular benefit in allowing the st_mode from one file being applied to another, without masking out the non-permission bits. I doubt this is a common programming idiom. All that said, I have no problem with the choice of option 1 if that is what everyone believes is the best way. |
(0004597) agadmin (administrator) 2019-10-07 15:15 |
Interpretation proposed: 7 October 2019 |
(0004643) agadmin (administrator) 2019-11-11 12:19 |
Interpretation Approved: 11 Nov 2019 |
![]() |
|||
Date Modified | Username | Field | Change |
2019-09-04 09:16 | geoffclare | New Issue | |
2019-09-04 09:16 | geoffclare | Name | => Geoff Clare |
2019-09-04 09:16 | geoffclare | Organization | => The Open Group |
2019-09-04 09:16 | geoffclare | Section | => chmod() |
2019-09-04 09:16 | geoffclare | Page Number | => 665 |
2019-09-04 09:16 | geoffclare | Line Number | => 22780 |
2019-09-04 09:16 | geoffclare | Interp Status | => --- |
2019-09-04 17:10 | shware_systems | Note Added: 0004547 | |
2019-09-04 22:40 | kre | Note Added: 0004548 | |
2019-09-04 23:03 | kre | Note Added: 0004549 | |
2019-09-05 01:48 | shware_systems | Note Added: 0004550 | |
2019-09-05 03:46 | kre | Note Added: 0004551 | |
2019-09-05 04:04 | kre | Note Added: 0004552 | |
2019-09-05 08:47 | geoffclare | Note Added: 0004553 | |
2019-09-05 09:00 | geoffclare | Note Edited: 0004553 | |
2019-09-05 09:01 | geoffclare | Note Edited: 0004553 | |
2019-10-03 16:04 | Don Cragun | Note Added: 0004592 | |
2019-10-03 16:04 | Don Cragun | Status | New => Interpretation Required |
2019-10-03 16:04 | Don Cragun | Resolution | Open => Accepted As Marked |
2019-10-03 16:04 | Don Cragun | Final Accepted Text | => See Note: 0004592. |
2019-10-03 16:05 | Don Cragun | Tag Attached: tc3-2008 | |
2019-10-03 16:05 | Don Cragun | Interp Status | --- => Pending |
2019-10-03 16:20 | Don Cragun | Note Edited: 0004592 | |
2019-10-03 16:21 | Don Cragun | Note Edited: 0004592 | |
2019-10-03 16:28 | shware_systems | Note Added: 0004593 | |
2019-10-03 16:30 | eblake | Note Added: 0004594 | |
2019-10-04 21:57 | kre | Note Added: 0004595 | |
2019-10-07 15:15 | agadmin | Interp Status | Pending => Proposed |
2019-10-07 15:15 | agadmin | Note Added: 0004597 | |
2019-11-11 12:19 | agadmin | Interp Status | Proposed => Approved |
2019-11-11 12:19 | agadmin | Note Added: 0004643 | |
2019-12-05 11:20 | geoffclare | Status | Interpretation Required => Applied |
Mantis 1.1.6[^] Copyright © 2000 - 2008 Mantis Group |