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
> >> > >> > >> >
> >> > >> > >
> >> > >>
> >> > >
> >> >
> >>
> >
>
