Re: [PATCH v4] fs: introduce is_dot_or_dotdot helper for cleanup

[Matthew Wilcox <willy@infradead.org>]

On Tue, Jan 28, 2020 at 06:23:18PM -0700, Andreas Dilger wrote:
> On Jan 28, 2020, at 3:11 PM, Matthew Wilcox <willy@infradead.org> wrote:
> > I've tried to get Ted's opinion on this a few times with radio silence.
> > Or email is broken.  Anyone else care to offer an opinion?
> 
> Maybe I'm missing something, but I think the discussion of the len == 0
> case is now moot, since PATCH v6 (which is the latest version that I can
> find) is checking for "len >= 1" before accessing name[0]:

Regardless of _this_ patch, the question is "Should ext4 be checking
for filenames of zero length and reporting -EUCLEAN if it finds any?"
I believe the answer is yes, since it's clearly a corrupted filesystem,
but I may be missing something.

Thanks for your reply.

> >> fscrypt_get_symlink():
> >>       if (cstr.len == 0)
> >>                return ERR_PTR(-EUCLEAN);
> >> ext4_readdir():
> >> 	Does not currently check de->name_len.  I believe this check should
> >> 	be added to __ext4_check_dir_entry() because a zero-length directory
> >> 	entry can affect both encrypted and non-encrypted directory entries.
> >> dx_show_leaf():
> >> 	Same as ext4_readdir().  Should probably call ext4_check_dir_entry()?
> >> htree_dirblock_to_tree():
> >> 	Would be covered by a fix to ext4_check_dir_entry().
> >> f2fs_fill_dentries():
> >> 	if (de->name_len == 0) {
> >> 		...
> >> ubifs_readdir():
> >> 	Does not currently check de->name_len.  Also affects non-encrypted
> >> 	directory entries.
> >> 
> >> So of the six callers, two of them already check the dirent length for
> >> being zero, and four of them ought to anyway, but don't.  I think they
> >> should be fixed, but clearly we don't historically check for this kind
> >> of data corruption (strangely), so I don't think that's a reason to hold
> >> up this patch until the individual filesystems are fixed.