On Wed, Jan 11, 2017 at 05:07:29PM +0800, zhangyi (F) wrote:
>
> (1) The file we want to unlink have many hard links, but only one dcache
> entry in memory.
> (2) open this file, but it's inode->i_nlink read from disk was 1 (too low).
> (3) some one call rename and drop it's i_nlink to zero.
> (4) it's inode is still in use and do not destroy (not closed), at the same
> time,
> some others open it's hard link and create a dcache entry.
> (5) call rename again and it's i_nlink will still underflow and cause memory
> corruption.
Do you have reproducers that make it easy to reproduce situations like
this? (It shouldn't be hard to write, but if you have them already
will save me some effort. :-)
If we ever get passed an inode to ext4_file_open() where i_nlink is
zero, we can declare the file system is corrupt by calling
ext4_error() to report the problem.
Similarly, whenever we are passed a dentry pointing to an inode for
link, unlink, rename, and other methods in the inode_operations
structure, by definition the file system is corrupt, and again we
should report this using ext4_error().
So I don't think we should think of this as adding "underflow
protection"; instead we should think of it as adding more aggressive
detection of file system inconsistencies. If there is dentry which is
valid, and pointing at an inode where n_links is zero, something has
gone seriously wrong. So we should call ext4_error() to report the
file system inconsistency, and then return EFSCORRUPTED (aka EUCLEAN).
Since we would be doing this in a number of places, we should probably
add an inline function:
static inline int ext4_validate_dentry(struct dentry *dentry);
which returns 0 if the dentry is valid, and calls ext4_error_inode()
and returns -EFSCORRUPTED if the dentry points to an inode with a zero
i_nlink.
(Note: it's valid for i_nlinks to be zero if the system call started
with a file descriptor, such as read(2) or write(2) operating on a
file which is still deleted but has open file descriptors. But if the
user has passed a pathname to the system call, such as in the case of
open(), rename(), unlink(), rmdir(), etc, then the dentry had better
be pointing at an inode with a non-zero i_nlink call. We need to be a
bit careful if the method function could be called by both a pathname
and file descriptor variant of the system call --- for example
fsetxattr(2) and setxattr(2); we won't be able to use
ext4_validate_dentry() for those inode_operations calls.)
Cheers,
- Ted
