656461 – cifs: fix problems with filehandle management and reporting of writeback errors

RHEL Engineering is moving the tracking of its product development work on RHEL 6 through RHEL 9 to Red Hat Jira (issues.redhat.com). If you're a Red Hat customer, please continue to file support cases via the Red Hat customer portal. If you're not, please head to the "RHEL project" in Red Hat Jira and file new tickets here. Individual Bugzilla bugs in the statuses "NEW", "ASSIGNED", and "POST" are being migrated throughout September 2023. Bugs of Red Hat partners with an assigned Engineering Partner Manager (EPM) are migrated in late September as per pre-agreed dates. Bugs against components "kernel", "kernel-rt", and "kpatch" are only migrated if still in "NEW" or "ASSIGNED". If you cannot log in to RH Jira, please consult article #7032570. That failing, please send an e-mail to the RH Jira admins at rh-issues@redhat.com to troubleshoot your issue as a user management inquiry. The email creates a ServiceNow ticket with Red Hat. Individual Bugzilla bugs that are migrated will be moved to status "CLOSED", resolution "MIGRATED", and set with "MigratedToJIRA" in "Keywords". The link to the successor Jira issue will be found under "Links", have a little "two-footprint" icon next to it, and direct you to the "RHEL project" in Red Hat Jira (issue links are of type "https://issues.redhat.com/browse/RHEL-XXXX", where "X" is a digit). This same link will be available in a blue banner at the top of the page informing you that that bug has been migrated.

Bug 656461 - cifs: fix problems with filehandle management and reporting of writeback errors

Summary: cifs: fix problems with filehandle management and reporting of writeback errors

Keywords:
Status:	CLOSED ERRATA
Alias:	None
Product:	Red Hat Enterprise Linux 6
Classification:	Red Hat
Component:	kernel
Sub Component:
Version:	6.0
Hardware:	Unspecified
OS:	Unspecified
Priority:	medium
Severity:	medium
Target Milestone:	rc
Target Release:	---
Assignee:	Jeff Layton
QA Contact:	yanfu,wang
Docs Contact:
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+	depends on / blocked

Reported:	2010-11-23 19:23 UTC by Jeff Layton
Modified:	2011-05-19 12:30 UTC (History)
CC List:	7 users (show)
Fixed In Version:	kernel-2.6.32-112.el6
Doc Type:	Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed:	2011-05-19 12:30:16 UTC
Target Upstream Version:
Embargoed:

Attachments	(Terms of Use)

Links
System	ID	Private	Priority	Status	Summary	Last Updated
Red Hat Product Errata	RHSA-2011:0542	0	normal	SHIPPED_LIVE	Important: Red Hat Enterprise Linux 6.1 kernel security, bug fix and enhancement update	2011-05-19 11:58:07 UTC

Description Jeff Layton 2010-11-23 19:23:43 UTC

CIFS has a really goofy way of dealing with open files on the server. They're tracked via a data structure attached to filp->private_data and those data structures are placed on a list that hangs from the inode.

When another process needs an open filehandle on the server, the code will often walk this list and "borrow" one from an existing open file. That's fine, but when it does this, it does no real refcounting. If the filp gets closed, then it waits a "little while" for the refcount to go low and eventually (after 2s) gives up and closes the file anyway.

Obviously this is racy and broken, and can lead to data integrity problems. I pushed a patchset upstream to fix many of these problems a month or two ago. This bug is to track backporting that for RHEL6.1.

Comment 1 RHEL Program Management 2010-11-23 21:09:42 UTC

This request was evaluated by Red Hat Product Management for inclusion
in a Red Hat Enterprise Linux maintenance release. Product Management has 
requested further review of this request by Red Hat Engineering, for potential
inclusion in a Red Hat Enterprise Linux Update release for currently deployed 
products. This request is not yet committed for inclusion in an Update release.

Comment 4 yanfu,wang 2010-12-01 07:38:19 UTC

hi Jeff,
Could you provide some basic test to cover it? thanks.

Comment 7 Aristeu Rozanski 2011-02-03 15:28:47 UTC

Patch(es) available on kernel-2.6.32-112.el6

Comment 11 yanfu,wang 2011-03-10 10:25:51 UTC

hi Jeff,
I can't find related change in fs/cifs/dir.c for the below patches when I unpack sources and apply patches from <specfile>:
http://patchwork.usersys.redhat.com/patch/30035/
http://patchwork.usersys.redhat.com/patch/30037/
http://patchwork.usersys.redhat.com/patch/30044/
http://patchwork.usersys.redhat.com/patch/30050/
http://patchwork.usersys.redhat.com/patch/30051/

Comment 12 yanfu,wang 2011-03-10 10:28:33 UTC

(In reply to comment #11)
> hi Jeff,
> I can't find related change in fs/cifs/dir.c for the below patches when I
> unpack sources and apply patches from <specfile>:
> http://patchwork.usersys.redhat.com/patch/30035/
> http://patchwork.usersys.redhat.com/patch/30037/
> http://patchwork.usersys.redhat.com/patch/30044/
> http://patchwork.usersys.redhat.com/patch/30050/
> http://patchwork.usersys.redhat.com/patch/30051/

you could login ibm-js22-vios-02-lp2.rhts.eng.bos.redhat.com with root/redhat if you want to check, the build dir is /root/rpmbuild/BUILD/kernel-2.6.32-119.el6/linux-2.6.32-119.el6.ppc64

Comment 13 Jeff Layton 2011-03-10 12:43:48 UTC

I did a spot check of the sources on that machine and those patches appear to have been applied. For instance, for:

    http://patchwork.usersys.redhat.com/patch/30051/

...the cifs_sb_active and cifs_sb_deactive functions are in cifsfs.c in the source tree. Perhaps if you can clarify what you're trying to check for and how you're performing that check I can try to help more?

Comment 14 yanfu,wang 2011-03-11 01:38:24 UTC

(In reply to comment #13)
> I did a spot check of the sources on that machine and those patches appear to
> have been applied. For instance, for:
> 
>     http://patchwork.usersys.redhat.com/patch/30051/
> 
> ...the cifs_sb_active and cifs_sb_deactive functions are in cifsfs.c in the
> source tree. Perhaps if you can clarify what you're trying to check for and how
> you're performing that check I can try to help more?

hi Jeff,
I mean the modification in fs/cifs/dir.c source file, for example, http://patchwork.usersys.redhat.com/patch/30051/
<snip>
diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
index 2960eb3..9f0536e 100644
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -132,8 +132,7 @@  cifs_bp_rename_retry:
 
 struct cifsFileInfo *
 cifs_new_fileinfo(struct inode *newinode, __u16 fileHandle, struct file *file,
-		  struct vfsmount *mnt, struct tcon_link *tlink,
-		  unsigned int oflags, __u32 oplock)
+		  struct tcon_link *tlink, unsigned int oflags, __u32 oplock)
 {
 	struct dentry *dentry = file->f_path.dentry;
 	struct cifsFileInfo *pCifsFile;
@@ -147,7 +146,6 @@  cifs_new_fileinfo(struct inode *newinode, __u16 fileHandle, struct file *file,
 	pCifsFile->pid = current->tgid;
 	pCifsFile->uid = current_fsuid();
 	pCifsFile->dentry = dget(dentry);
-	pCifsFile->mnt = mnt;
 	pCifsFile->pfile = file;
 	pCifsFile->invalidHandle = false;
 	pCifsFile->closePend = false;
@@ -484,8 +482,7 @@  cifs_create_set_dentry:
 		}
 
 		pfile_info = cifs_new_fileinfo(newinode, fileHandle, filp,
-						nd->path.mnt, tlink, oflags,
-						oplock);
+						tlink, oflags, oplock);
<snip>

But I can't find the added line like "struct tcon_link *tlink, unsigned int oflags, __u32 oplock)".

Sorry if my expression is unclear.

Comment 15 Jeff Layton 2011-03-11 01:49:56 UTC

That's because a later patch moves the entire cifs_new_fileinfo function
to file.c.

If your intent is to check whether the patches have been applied correctly,
then it might make more sense to try and back the patches out in reverse
order. Many of these patches alter the same pieces of code in different
ways.

Comment 16 yanfu,wang 2011-03-11 02:36:18 UTC

(In reply to comment #15)
> That's because a later patch moves the entire cifs_new_fileinfo function
> to file.c.
> 
> If your intent is to check whether the patches have been applied correctly,
> then it might make more sense to try and back the patches out in reverse
> order. Many of these patches alter the same pieces of code in different
> ways.

Thanks Jeff, I've got it. I finish basic function testing also, results passed:
https://beaker.engineering.redhat.com/jobs/60763
https://beaker.engineering.redhat.com/jobs/60762
https://beaker.engineering.redhat.com/jobs/60761
https://beaker.engineering.redhat.com/jobs/60735

Comment 17 errata-xmlrpc 2011-05-19 12:30:16 UTC

An advisory has been issued which should help the problem
described in this bug report. This report is therefore being
closed with a resolution of ERRATA. For more information
on therefore solution and/or where to find the updated files,
please follow the link below. You may reopen this bug report
if the solution does not work for you.

http://rhn.redhat.com/errata/RHSA-2011-0542.html

Note You need to log in before you can comment on or make changes to this bug.