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

Reply via email to