On Fri, 24 Feb 2012 11:59:25 -0600
Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx> wrote:
> On Thu, Feb 23, 2012 at 8:37 AM, Jeff Layton <jlayton@xxxxxxxxxx> wrote:
> > The cifs code will attempt to open files on lookup under certain
> > circumstances. What happens though if we find that the file we opened
> > was actually a FIFO or other special file?
> >
> > Currently, the open filehandle just ends up being leaked leading to
> > a dentry refcount mismatch and oops on umount. Fix this by having the
> > code close the filehandle on the server if it turns out not to be a
> > regular file. While we're at it, change this spaghetti if statement
> > into a switch too.
> >
> > Cc: stable@xxxxxxxxxxxxxxx
> > Reported-by: CAI Qian <caiqian@xxxxxxxxxx>
> > Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
> > ---
> > fs/cifs/dir.c | 20 ++++++++++++++++++--
> > 1 files changed, 18 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
> > index 63a196b..bc7e244 100644
> > --- a/fs/cifs/dir.c
> > +++ b/fs/cifs/dir.c
> > @@ -584,10 +584,26 @@ cifs_lookup(struct inode *parent_dir_inode, struct dentry *direntry,
> > * If either that or op not supported returned, follow
> > * the normal lookup.
> > */
> > - if ((rc == 0) || (rc == -ENOENT))
> > + switch (rc) {
> > + case 0:
> > + /*
> > + * The server may allow us to open things like
> > + * FIFOs, but the client isn't set up to deal
> > + * with that. If it's not a regular file, just
> > + * close it and proceed as if it were a normal
> > + * lookup.
> > + */
> > + if (newInode && !S_ISREG(newInode->i_mode)) {
> > + CIFSSMBClose(xid, pTcon, fileHandle);
> > + break;
> > + }
> > + case -ENOENT:
> > posix_open = true;
> > - else if ((rc == -EINVAL) || (rc != -EOPNOTSUPP))
> > + case -EOPNOTSUPP:
> > + break;
> > + default:
> > pTcon->broken_posix_open = true;
> > + }
> > }
> > if (!posix_open)
> > rc = cifs_get_inode_info_unix(&newInode, full_path,
> > --
> > 1.7.7.6
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> > the body of a message to majordomo@xxxxxxxxxxxxxxx
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> One more thing, should CIFSPOSIXDelFile should be called to match
> CIFSPOSIXOpen or CIFSMBClose is just as good enough?
I don't think so...
If the FIFO was already there, then you clearly don't want to remove
it. Note that in this case, we're not failing anything -- just getting
rid of the open filehandle and corresponding CIFS open when we realize
that it's not a normal file.
--
Jeff Layton <jlayton@xxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
