In my opinion 0, if you are asking that, but it is strictly not conforming the standard.
Posix says that "Applications shall specify exactly one of the first five...", so there is no correct standard conforming way. All five would be wrong, imho. -Jukka Petro Karashchenko kirjoitti perjantai 1. huhtikuuta 2022: > Yes Jukka what you are saying is absolutely correct. The main item under > discussion are the drivers that are pure ioctl (that means do not have > neither read nor write handlers). Should RD or WR flag be passed to open > call in such case of or 0 should be passed. > > Best regards, > Petro > > On Fri, Apr 1, 2022, 6:54 PM Jukka Laitinen <[email protected]> wrote: > > > Different posix implementations have different values for these flags, so > > I think it is ok not to have the same as what linux has. > > > > Posix (2017) specifies thar exactly one of the following is provided for > > open: O_EXEC, O_RDWR, O_RDONLY, O_SEARCH and O_WRONLY, and other flags are > > bitwise OR'd to that. The spec says nothing about that you can just give > > "0" afaik, it just happens to be that in Linux O_RDONLY happens to be 0. > > > > Maybe just fix the open having O_RDONLY in places where you really open > > the file as read-only, O_WRONLY for write only and O? That should be > > according to the standard at least. > > > > Just my 2 cents > > > > - Jukka > > > > > > > > Alan Carvalho de Assis kirjoitti perjantai 1. huhtikuuta 2022: > > > More about it here: > > > > > https://stackoverflow.com/questions/61923703/how-to-make-sense-of-o-rdonly-0 > > > > > > So, I agree with the comment that said "calling it flag is misnomer > > > and misleading" > > > > > > On Unix/Linux O_RDONLY | O_WRONLY != O_RDWR, on NuttX is it explicitly > > this way: > > > > > > #define O_RDWR (O_RDOK|O_WROK) /* Open for both read & write access > > */ > > > > > > Now, I'm also confuse about the right thing to do: > > > > > > 1) Fix include/fcntl.h to follow Unix/Linux > > > 2) Keep things we they are and don't accept opening a file if it > > > doesn't have RDONLY flag (and change all the drivers, event ioctl only > > > drivers, to include reading flag) > > > 3) Remove the flag checking. > > > > > > Probably we should do 1) because NuttX follows Unix/Linux approach, > > > although I agree that Unix/Linux are completely non-sense on this > > > subject, oflag should be a flag like it is on NuttX :-) > > > > > > BR, > > > > > > Alan > > > > > > On 4/1/22, Alan Carvalho de Assis <[email protected]> wrote: > > > > Hmm, I think oflag equal 0 on Unix(so MacOS)/Linux works because of it: > > > > > > > > #define O_RDONLY 00000000 > > > > > > > > So, in fact on Linux/Mac when we are opening a file with oflag 0 we > > > > are opening it for reading only. > > > > > > > > On NuttX the value 0 is not defined, O_RDONLY is 1: > > > > > > > > #define O_RDONLY (1 << 0) > > > > > > > > BR, > > > > > > > > Alan > > > > > > > > On 4/1/22, Petro Karashchenko <[email protected]> wrote: > > > >> Hello, > > > >> > > > >> Here I'm talking not about driver registration permission, but more > > about > > > >> the "oflag" parameter to "open()" call. > > > >> > > > >> I just tried a quick example on MAC > > > >> > > > >> #include <fcntl.h> > > > >> #include <stdio.h> > > > >> > > > >> int main(void) > > > >> { > > > >> int fd = open("test.txt", 0); > > > >> if (fd < 0) > > > >> printf("A\n"); > > > >> else > > > >> printf("B\n"); > > > >> > > > >> return 0; > > > >> } > > > >> > > > >> The "B" is printed if the file exists. If you know the system that > > will > > > >> run > > > >> this sample code and will print "A", please let me know. > > > >> > > > >> Best regards, > > > >> Petro > > > >> > > > >> пт, 1 квіт. 2022 р. о 16:27 Alan Carvalho de Assis <[email protected] > > > > > > >> пише: > > > >> > > > >>> I think the device file shouldn't be created with permission 000. > > > >>> > > > >>> Look inside your Linux /dev all device files have RW permission for > > > >>> root, some give only R for group and others. > > > >>> > > > >>> So, probably we need to fix the device register creation, not > > removing > > > >>> the flag check. > > > >>> > > > >>> BR, > > > >>> > > > >>> Alan > > > >>> > > > >>> On 4/1/22, Xiang Xiao <[email protected]> wrote: > > > >>> > It's better to check ioctl callback too since ioctl means the > > driver > > > >>> > has > > > >>> > the compatibility of read(i)and write(o). > > > >>> > > > > >>> > On Fri, Apr 1, 2022 at 9:15 PM Petro Karashchenko < > > > >>> > [email protected]> wrote: > > > >>> > > > > >>> >> So Alan do you suggest to remove inode_checkflags? > > > >>> >> > > > >>> >> On Fri, Apr 1, 2022, 4:13 PM Alan Carvalho de Assis > > > >>> >> <[email protected]> > > > >>> >> wrote: > > > >>> >> > > > >>> >> > Hi Petro, > > > >>> >> > > > > >>> >> > I saw your PR #1117 but I think opening a device file with flag > > 0 > > > >>> >> > is > > > >>> >> > not correct, please see the open man-pages: > > > >>> >> > > > > >>> >> > alan@dev:/tmp$ man 2 open > > > >>> >> > > > > >>> >> > The argument flags must include one of the following > > > >>> >> > access > > > >>> >> > modes: O_RDONLY, O_WRONLY, or > > > >>> >> > O_RDWR. These request opening the file read-only, > > > >>> >> > write-only, > > > >>> >> > or read/write, respectively. > > > >>> >> > > > > >>> >> > Also the opengroup say something similar: > > > >>> >> > > > > >>> >> > > > https://pubs.opengroup.org/onlinepubs/9699919799/functions/open.html > > > >>> >> > > > > >>> >> > "Values for oflag are constructed by a bitwise-inclusive OR of > > > >>> >> > flags > > > >>> >> > from the following list, defined in <fcntl.h>. Applications > > shall > > > >>> >> > specify exactly one of the first five values (file access modes) > > > >>> >> > below > > > >>> >> > in the value of oflag:" > > > >>> >> > > > > >>> >> > The man pages uses "MUST", the OpenGroups uses "SHALL", but > > > >>> >> > according > > > >>> >> > to RFC2119 they are equivalents: > > > >>> >> > > > > >>> >> > https://www.ietf.org/rfc/rfc2119.txt > > > >>> >> > > > > >>> >> > BR, > > > >>> >> > > > > >>> >> > Alan > > > >>> >> > > > > >>> >> > On 4/1/22, Petro Karashchenko <[email protected]> > > wrote: > > > >>> >> > > Hi, > > > >>> >> > > > > > >>> >> > > I want to resume this thread again because after reexamined > > code > > > >>> >> > carefully > > > >>> >> > > I found that VFS layer has an API > > > >>> >> > > > > > >>> >> > > int inode_checkflags(FAR struct inode *inode, int oflags) > > > >>> >> > > { > > > >>> >> > > if (((oflags & O_RDOK) != 0 && !inode->u.i_ops->read) || > > > >>> >> > > ((oflags & O_WROK) != 0 && !inode->u.i_ops->write)) > > > >>> >> > > { > > > >>> >> > > return -EACCES; > > > >>> >> > > } > > > >>> >> > > else > > > >>> >> > > { > > > >>> >> > > return OK; > > > >>> >> > > } > > > >>> >> > > } > > > >>> >> > > > > > >>> >> > > That checks if read and write handlers are available, so all > > our > > > >>> >> > discussion > > > >>> >> > > about R/W mode for IOCTL does not make any sense. We either > > need > > > >>> >> > > to > > > >>> >> > remove > > > >>> >> > > this check or register VFS nodes with proper permissions and > > open > > > >>> >> > > files > > > >>> >> > > with correct flags. So if the driver does not have neither > > read > > > >>> >> > > nor > > > >>> >> write > > > >>> >> > > handlers the "0000" mode should be used and "0" should be used > > > >>> during > > > >>> >> > > opening of a file. Or we need to remove "inode_checkflags()". > > > >>> >> > > > > > >>> >> > > Best regards, > > > >>> >> > > Petro > > > >>> >> > > > > > >>> >> > > пт, 28 січ. 2022 р. о 15:11 Petro Karashchenko > > > >>> >> > > <[email protected]> > > > >>> >> > > пише: > > > >>> >> > > > > > >>> >> > >> I see. Thank you for the feedback. I will rework changes to > > get > > > >>> back > > > >>> >> > >> read permissions. > > > >>> >> > >> > > > >>> >> > >> Best regards, > > > >>> >> > >> Petro > > > >>> >> > >> > > > >>> >> > >> пт, 28 січ. 2022 р. о 14:41 Alan Carvalho de Assis > > > >>> >> > >> <[email protected] > > > >>> >> > > > > >>> >> > >> пише: > > > >>> >> > >> > > > > >>> >> > >> > Hi Petro, > > > >>> >> > >> > > > > >>> >> > >> > The read permission is needed even when you just want to > > open > > > >>> >> > >> > a > > > >>> >> file: > > > >>> >> > >> > > > > >>> >> > >> > $ vim noreadfile > > > >>> >> > >> > > > > >>> >> > >> > $ chmod 0000 noreadfile > > > >>> >> > >> > > > > >>> >> > >> > $ ls -l noreadfile > > > >>> >> > >> > ---------- 1 user user 5 jan 28 09:24 noreadfile > > > >>> >> > >> > > > > >>> >> > >> > $ cat noreadfile > > > >>> >> > >> > cat: noreadfile: Permission denied > > > >>> >> > >> > > > > >>> >> > >> > You can even try to create a C program just to open it, > > and it > > > >>> >> > >> > will > > > >>> >> > >> > fail. > > > >>> >> > >> > > > > >>> >> > >> > See the man page of open function: > > > >>> >> > >> > > > > >>> >> > >> > The argument flags *must* include one of the > > following > > > >>> >> access > > > >>> >> > >> > modes: O_RDONLY, O_WRONLY, or > > > >>> >> > >> > O_RDWR. These request opening the file read-only, > > > >>> >> write-only, > > > >>> >> > >> > or read/write, respectively. > > > >>> >> > >> > > > > >>> >> > >> > This man page makes it clear you must include an access > > mode, > > > >>> >> > >> > but > > > >>> >> > >> > I > > > >>> >> > >> > passed 0 to the access mode flag of open() and it was > > > >>> >> > >> > accepted, > > > >>> >> > >> > but > > > >>> >> > >> > when the file has permission 0000 it returns -EPERM: > > "Failed > > > >>> >> > >> > to > > > >>> >> > >> > open > > > >>> >> > >> > file: error -1" > > > >>> >> > >> > > > > >>> >> > >> > BR, > > > >>> >> > >> > > > > >>> >> > >> > Alan > > > >>> >> > >> > > > > >>> >> > >> > On 1/28/22, Petro Karashchenko < > > [email protected]> > > > >>> >> wrote: > > > >>> >> > >> > > Hello, > > > >>> >> > >> > > > > > >>> >> > >> > > Yes, but how does this relate to "0000" mode for > > > >>> >> > "register_driver()"? > > > >>> >> > >> > > Maybe you can describe some use case so it will become > > more > > > >>> >> > >> > > clear? > > > >>> >> > >> > > Currently ioctl works fine if driver is registered with > > > >>> >> > >> > > "0000" > > > >>> >> > >> permission > > > >>> >> > >> > > mode. > > > >>> >> > >> > > > > > >>> >> > >> > > Best regards, > > > >>> >> > >> > > Petro > > > >>> >> > >> > > > > > >>> >> > >> > > пт, 28 січ. 2022 р. о 11:39 Xiang Xiao > > > >>> >> > >> > > <[email protected] > > > >>> >> > > > > >>> >> > >> пише: > > > >>> >> > >> > >> > > > >>> >> > >> > >> If we want to do the correct permission check, the ioctl > > > >>> >> > >> > >> handler > > > >>> >> > >> needs to > > > >>> >> > >> > >> check R/W bit by itself based on how the ioctl is > > > >>> >> > >> > >> implemented. > > > >>> >> > >> > >> Or follow up how Linux encode the needed permission into > > > >>> >> > >> > >> each > > > >>> >> > IOCTL: > > > >>> >> > >> > >> > > > >>> >> > >> > > > >>> >> > > > > >>> >> > > > >>> > > https://github.com/torvalds/linux/blob/master/include/uapi/asm-generic/ioctl.h#L85-L91 > > > >>> >> > >> > >> and let's VFS layer do the check for each driver. > > > >>> >> > >> > >> > > > >>> >> > >> > >> On Fri, Jan 28, 2022 at 5:14 PM Petro Karashchenko < > > > >>> >> > >> > >> [email protected]> wrote: > > > >>> >> > >> > >> > > > >>> >> > >> > >> > Hello team, > > > >>> >> > >> > >> > > > > >>> >> > >> > >> > Recently I have noticed that there are many places in > > > >>> >> > >> > >> > code > > > >>> >> where > > > >>> >> > >> > >> > register_driver() is called with non-zero mode with > > file > > > >>> >> > operation > > > >>> >> > >> > >> > structures that have neither read nor write APIs > > > >>> implemented. > > > >>> >> For > > > >>> >> > >> > >> > example "ret = register_driver(path, &opamp_fops, > > 0444, > > > >>> >> > >> > >> > dev);" > > > >>> >> > >> > >> > while > > > >>> >> > >> > >> > opamp_fops has only "opamp_open", "opamp_close" and > > > >>> >> "opamp_ioctl" > > > >>> >> > >> > >> > implemented. I made a PR to fix it > > > >>> >> > >> > >> > https://github.com/apache/incubator-nuttx/pull/5347 > > and > > > >>> >> > >> > >> > change > > > >>> >> > >> > >> > mode > > > >>> >> > >> > >> > from "0444" to "0000", but want to ask if anyone sees > > any > > > >>> >> > drawback > > > >>> >> > >> in > > > >>> >> > >> > >> > such an approach? Maybe I'm missing something? > > > >>> >> > >> > >> > > > > >>> >> > >> > >> > Best regards, > > > >>> >> > >> > >> > Petro > > > >>> >> > >> > >> > > > > >>> >> > >> > > > > > >>> >> > >> > > > >>> >> > > > > > >>> >> > > > > >>> >> > > > >>> > > > > >>> > > > >> > > > > > > > >
