| Message ID | 20171215204832.12764-1-willy@infradead.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
thoughts on whether stable candidate? On Fri, Dec 15, 2017 at 2:48 PM, Matthew Wilcox <willy@infradead.org> wrote: > From: Matthew Wilcox <mawilcox@microsoft.com> > > If cifs_zap_mapping() returned an error, we would return without putting > the xid that we got earlier. Restructure cifs_file_strict_mmap() and > cifs_file_mmap() to be more similar to each other and have a single > point of return that always puts the xid. > > Signed-off-by: Matthew Wilcox <mawilcox@microsoft.com> > --- > fs/cifs/file.c | 26 ++++++++++++-------------- > 1 file changed, 12 insertions(+), 14 deletions(-) > > diff --git a/fs/cifs/file.c b/fs/cifs/file.c > index df9f682708c6..3a85df2a9baf 100644 > --- a/fs/cifs/file.c > +++ b/fs/cifs/file.c > @@ -3471,20 +3471,18 @@ static const struct vm_operations_struct cifs_file_vm_ops = { > > int cifs_file_strict_mmap(struct file *file, struct vm_area_struct *vma) > { > - int rc, xid; > + int xid, rc = 0; > struct inode *inode = file_inode(file); > > xid = get_xid(); > > - if (!CIFS_CACHE_READ(CIFS_I(inode))) { > + if (!CIFS_CACHE_READ(CIFS_I(inode))) > rc = cifs_zap_mapping(inode); > - if (rc) > - return rc; > - } > - > - rc = generic_file_mmap(file, vma); > - if (rc == 0) > + if (!rc) > + rc = generic_file_mmap(file, vma); > + if (!rc) > vma->vm_ops = &cifs_file_vm_ops; > + > free_xid(xid); > return rc; > } > @@ -3494,16 +3492,16 @@ int cifs_file_mmap(struct file *file, struct vm_area_struct *vma) > int rc, xid; > > xid = get_xid(); > + > rc = cifs_revalidate_file(file); > - if (rc) { > + if (rc) > cifs_dbg(FYI, "Validation prior to mmap failed, error=%d\n", > rc); > - free_xid(xid); > - return rc; > - } > - rc = generic_file_mmap(file, vma); > - if (rc == 0) > + if (!rc) > + rc = generic_file_mmap(file, vma); > + if (!rc) > vma->vm_ops = &cifs_file_vm_ops; > + > free_xid(xid); > return rc; > } > -- > 2.15.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-cifs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Dec 15, 2017 at 02:51:10PM -0600, Steve French wrote:
> thoughts on whether stable candidate?
Ehm ... your call. I don't know how serious leaking an XID is. From
reading the code, it doesn't look too serious.
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
No - that is not serious - but ... trivial low risk patch ... I would lean toward no ... unless a customer reported it On Fri, Dec 15, 2017 at 2:53 PM, Matthew Wilcox <willy@infradead.org> wrote: > On Fri, Dec 15, 2017 at 02:51:10PM -0600, Steve French wrote: >> thoughts on whether stable candidate? > > Ehm ... your call. I don't know how serious leaking an XID is. From > reading the code, it doesn't look too serious.
On Fri, Dec 15, 2017 at 02:54:54PM -0600, Steve French wrote: > No - that is not serious - but ... trivial low risk patch ... > > I would lean toward no ... unless a customer reported it No customer report; code inspection on my part. -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
merged into cifs-2.6.git for-next On Fri, Dec 15, 2017 at 2:48 PM, Matthew Wilcox <willy@infradead.org> wrote: > From: Matthew Wilcox <mawilcox@microsoft.com> > > If cifs_zap_mapping() returned an error, we would return without putting > the xid that we got earlier. Restructure cifs_file_strict_mmap() and > cifs_file_mmap() to be more similar to each other and have a single > point of return that always puts the xid. > > Signed-off-by: Matthew Wilcox <mawilcox@microsoft.com> > --- > fs/cifs/file.c | 26 ++++++++++++-------------- > 1 file changed, 12 insertions(+), 14 deletions(-) > > diff --git a/fs/cifs/file.c b/fs/cifs/file.c > index df9f682708c6..3a85df2a9baf 100644 > --- a/fs/cifs/file.c > +++ b/fs/cifs/file.c > @@ -3471,20 +3471,18 @@ static const struct vm_operations_struct cifs_file_vm_ops = { > > int cifs_file_strict_mmap(struct file *file, struct vm_area_struct *vma) > { > - int rc, xid; > + int xid, rc = 0; > struct inode *inode = file_inode(file); > > xid = get_xid(); > > - if (!CIFS_CACHE_READ(CIFS_I(inode))) { > + if (!CIFS_CACHE_READ(CIFS_I(inode))) > rc = cifs_zap_mapping(inode); > - if (rc) > - return rc; > - } > - > - rc = generic_file_mmap(file, vma); > - if (rc == 0) > + if (!rc) > + rc = generic_file_mmap(file, vma); > + if (!rc) > vma->vm_ops = &cifs_file_vm_ops; > + > free_xid(xid); > return rc; > } > @@ -3494,16 +3492,16 @@ int cifs_file_mmap(struct file *file, struct vm_area_struct *vma) > int rc, xid; > > xid = get_xid(); > + > rc = cifs_revalidate_file(file); > - if (rc) { > + if (rc) > cifs_dbg(FYI, "Validation prior to mmap failed, error=%d\n", > rc); > - free_xid(xid); > - return rc; > - } > - rc = generic_file_mmap(file, vma); > - if (rc == 0) > + if (!rc) > + rc = generic_file_mmap(file, vma); > + if (!rc) > vma->vm_ops = &cifs_file_vm_ops; > + > free_xid(xid); > return rc; > } > -- > 2.15.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-cifs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/cifs/file.c b/fs/cifs/file.c index df9f682708c6..3a85df2a9baf 100644 --- a/fs/cifs/file.c +++ b/fs/cifs/file.c @@ -3471,20 +3471,18 @@ static const struct vm_operations_struct cifs_file_vm_ops = { int cifs_file_strict_mmap(struct file *file, struct vm_area_struct *vma) { - int rc, xid; + int xid, rc = 0; struct inode *inode = file_inode(file); xid = get_xid(); - if (!CIFS_CACHE_READ(CIFS_I(inode))) { + if (!CIFS_CACHE_READ(CIFS_I(inode))) rc = cifs_zap_mapping(inode); - if (rc) - return rc; - } - - rc = generic_file_mmap(file, vma); - if (rc == 0) + if (!rc) + rc = generic_file_mmap(file, vma); + if (!rc) vma->vm_ops = &cifs_file_vm_ops; + free_xid(xid); return rc; } @@ -3494,16 +3492,16 @@ int cifs_file_mmap(struct file *file, struct vm_area_struct *vma) int rc, xid; xid = get_xid(); + rc = cifs_revalidate_file(file); - if (rc) { + if (rc) cifs_dbg(FYI, "Validation prior to mmap failed, error=%d\n", rc); - free_xid(xid); - return rc; - } - rc = generic_file_mmap(file, vma); - if (rc == 0) + if (!rc) + rc = generic_file_mmap(file, vma); + if (!rc) vma->vm_ops = &cifs_file_vm_ops; + free_xid(xid); return rc; }