Bug 577378 - NFSv3 file attributes are not updated by READDIRPLUS reply
Summary: NFSv3 file attributes are not updated by READDIRPLUS reply
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Linux 4
Classification: Red Hat
Component: kernel
Version: 4.8
Hardware: All
OS: Linux
urgent
urgent
Target Milestone: rc
: ---
Assignee: Jeff Layton
QA Contact: Petr Beňas
URL:
Whiteboard:
Depends On:
Blocks: 596372
TreeView+ depends on / blocked
 
Reported: 2010-03-26 19:58 UTC by Fabio Olive Leite
Modified: 2018-10-27 12:16 UTC (History)
9 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2011-02-16 15:43:16 UTC
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)
Patch backported from RHEL-5 (2.51 KB, patch)
2010-03-26 19:58 UTC, Fabio Olive Leite
no flags Details | Diff
getdents program to verify the bug and the fix (1.80 KB, text/x-csrc)
2010-03-26 20:00 UTC, Fabio Olive Leite
no flags Details


Links
System ID Private Priority Status Summary Last Updated
Red Hat Product Errata RHSA-2011:0263 0 normal SHIPPED_LIVE Important: Red Hat Enterprise Linux 4.9 kernel security and bug fix update 2011-02-16 15:14:55 UTC

Description Fabio Olive Leite 2010-03-26 19:58:39 UTC
Created attachment 402931 [details]
Patch backported from RHEL-5

Description of problem:

When a RHEL-4 NFSv3 client performs a READDIRPLUS operation at the server, the file attributes for existing cached files are not updated. In this case if an existing file name that points to a regular file gets changed into a symlink or a directory, it is not noticed until an actual stat() call forces an NFS GETATTR on the specific file.

Certain system calls, such as getdents(), return file type information that is based on existing cached file attributes. When a certain directory entry changes it's type, the parent directory contents are refreshed given its mtime change, but the actual directory entry information is not. So the getdents() call returns stale information, and this can keep happening for several hours, regardless of "noac" or "actimeo" settings, since nothing will cause each specific dentry to be updated even if the READDIRPLUS reply provides the needed information.

Certain core utilities, such as find(1), use getdents() and get fooled by this behavior.

This was fixed early on in RHEL-5 in a monster patch attributed to bug 242975. I found the specific upstream commit, the large RHEL-5 commit, and factored out and backported only the changes to nfs_readdir_lookup that are relevant.

Patch applies cleanly to latest RHEL-4 sources and fixes the issue. I'll also attach a tiny "getdents.c" program (from the getdents manpage) that can be used to verify this very easily. Notice a simple "ls -l" does not trigger it since ls(1) does an actual stat() on each directory entry.

Version-Release number of selected component (if applicable):

kernel-2.6.9-89.EL

How reproducible:

Always.

Steps to Reproduce:

1. Mount an NFS volume using RHEL-4, say on /mnt

2. Run the getdents program on a test directory containing a few entries, such as "getdents /mnt/test/"

3. Notice the file types listed for each directory entry

4. At the server or in another NFS client, change a certain directory entry to another type, such as "rm -f /mnt/test/foobar ; mkdir /mnt/test/foobar"

5. Run "getdents /mnt/test/" in the RHEL-4 client again, and notice that it still lists "/mnt/test/foobar" as a file, instead of a directory.

Example:

$ ./getdents /mnt/test/
--------------- nread=80 ---------------
i-node#  file type  d_reclen  d_off   d_name
 1261573  directory    24  796079690  .
 1261569  directory    24  817063672  ..
 1261576  symlink      32 2147483647  foobar

[ At the server, remove the symlink and create a file with the same name ]

$ ./getdents /mnt/test/
--------------- nread=80 ---------------
i-node#  file type  d_reclen  d_off   d_name
 1261573  directory    24  796079690  .
 1261569  directory    24  817063672  ..
 1261576  symlink      32 2147483647  foobar

[ The RHEL-4 NFS client did not perceive the change ]

$ stat /mnt/test/foobar 
  File: `/mnt/test/foobar'
  Size: 0    Blocks: 0    IO Block: 4096    regular empty file
...

[ Explicit stat() issues an explicit GETATTR ]

$ ./getdents /mnt/test/
--------------- nread=80 ---------------
i-node#  file type  d_reclen  d_off   d_name
 1261573  directory    24  796079690  .
 1261569  directory    24  817063672  ..
 1261576  regular      32 2147483647  foobar

[ Only after the explicit stat() the type was updated ]

It is easy to see in wireshark that the RHEL-4 NFS client does notice the change in the directory's mtime, and issues a READDIRPLUS, which does return updated information for "foobar", but the nfs_readdir_lookup() function does not use it.

Actual results:

getdents() returns stale type information in the dirent->d_type fields.

Expected results:

getdents() returns updated type information in the dirent->d_type fields.

Additional info:

Customer is also testing the patch.

Comment 1 Fabio Olive Leite 2010-03-26 20:00:39 UTC
Created attachment 402932 [details]
getdents program to verify the bug and the fix

This was taken from the getdents() manpage.

Comment 2 Fabio Olive Leite 2010-03-26 22:51:19 UTC
One aspect of this that just came to my mind, though, is that this issue contains two "root causes":

1. getdents() or readdir() return d_type information based on cached file attributes without causing a refresh of those attributes;

2. the kernel was not using the fresh data from the READDIRPLUS replies.

We fixed the second problem, but if we stop to think, the first one is still there.

I just mounted an NFS volume in both my patched RHEL-4 system and also on my Fedora 12 netbook with the "nordirplus" option, to disable the usage of READDIRPLUS. This immediately causes the issue to always reproduce. Surely it is obvious, but it is a problematic use case we should consider.

When we do a readdir() on a directory, we check if the mtime has changed, and if it has we get a listing of directory entries. It appears that the code implies that if a certain directory entry still has the same name, it will map to the same object, which is not correct.

Perhaps we should also ensure to return d_type of DT_UNKNOWN for each existing cached positive dentry if the directory attributes were updated after the file attributes of that dentry, since we cannot be sure if that dentry's attributes can be trusted.

I do not really know what to make of it. I believe it's been this way since "forever".

Fábio Olivé

Comment 4 Jeff Layton 2010-04-15 17:50:44 UTC
I've put Fabio's patch into my test kernels.(well, actually my own backport of the two upstream patches, that matches Fabio's backport):

    http://people.redhat.com/jlayton/

It would be helpful if you could confirm that they fix the problem. The other issue that still seems to be an upsteam problem should probably be cloned to a separate bug if you're interested in pursuing it.

Comment 7 RHEL Program Management 2010-04-16 19:41:08 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 12 Vivek Goyal 2010-05-27 15:02:35 UTC
Committed in 89.26.EL . RPMS are available at http://people.redhat.com/vgoyal/rhel4/

Comment 19 Petr Beňas 2010-11-30 13:43:35 UTC
Reproduced in 89.25.EL and verified in 89.26.EL.

Comment 21 errata-xmlrpc 2011-02-16 15:43: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-0263.html


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