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 <petro.karashche...@gmail.com> 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
> <petro.karashche...@gmail.com>
> пише:
>
>> 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 <acas...@gmail.com>
>> пише:
>> >
>> > 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 <petro.karashche...@gmail.com> 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 <xiaoxiang781...@gmail.com>
>> пише:
>> > >>
>> > >> 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 <
>> > >> petro.karashche...@gmail.com> 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
>> > >> >
>> > >
>>
>
