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 -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
