[PATCH] ovl: handle ENOENT on index lookup

[PATCH] ovl: handle ENOENT on index lookup

[Amir Goldstein]

Fixes: 359f392ca53e ("ovl: lookup index entry for copy up origin")
Cc: <stable@vger.kernel.org> # v4.13
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/overlayfs/namei.c | 4 ++++
 1 file changed, 4 insertions(+)

Miklos,

This is a complimentary/fixup patch to the patch currently at the tip of
overlayfs-next. I did not run into ENOENT in my tests, but it seems that
all other places in overlayfs that call lookup_one_len_unlocked() check
the ENOENT return value and treat it the same as negative dentry.

I suppose this could be expected from some file systems?

Amir.

diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
index 49441e8be722..a824e6b157f9 100644
--- a/fs/overlayfs/namei.c
+++ b/fs/overlayfs/namei.c
@@ -507,6 +507,10 @@ static struct dentry *ovl_lookup_index(struct dentry *dentry,
 	index = lookup_one_len_unlocked(name.name, ofs->indexdir, name.len);
 	if (IS_ERR(index)) {
 		err = PTR_ERR(index);
+		if (err == -ENOENT) {
+			index = NULL;
+			goto out;
+		}
 		pr_warn_ratelimited("overlayfs: failed inode index lookup (ino=%lu, key=%*s, err=%i);\n"
 				    "overlayfs: mount with '-o index=off' to disable inodes index.\n",
 				    d_inode(origin)->i_ino, name.len, name.name,
-- 
2.7.4

Re: [PATCH] ovl: handle ENOENT on index lookup

[Greg KH]

On Fri, Oct 20, 2017 at 02:01:10PM +0300, Amir Goldstein wrote:
> Fixes: 359f392ca53e ("ovl: lookup index entry for copy up origin")
> Cc: <stable@vger.kernel.org> # v4.13
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>

Why is there no changelog at all?  Not good, especially for a patch
wanting to be sent to the stable trees :(

thanks,

greg k-h

Re: [PATCH] ovl: handle ENOENT on index lookup

[Amir Goldstein]

On Fri, Oct 20, 2017 at 2:37 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Fri, Oct 20, 2017 at 02:01:10PM +0300, Amir Goldstein wrote:
>> Fixes: 359f392ca53e ("ovl: lookup index entry for copy up origin")
>> Cc: <stable@vger.kernel.org> # v4.13
>> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
>
> Why is there no changelog at all?  Not good, especially for a patch
> wanting to be sent to the stable trees :(
>

Yeh, sorry about that. I was hoping it'd get squashed into an earlier
patch which is on overlayfs-next of today and not yet pulled into linux-next.
Or rejected.
If that doesn't happen, I'll re-post with commit message.

Amir.

Re: [PATCH] ovl: handle ENOENT on index lookup

[Miklos Szeredi]

On Fri, Oct 20, 2017 at 1:01 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> Fixes: 359f392ca53e ("ovl: lookup index entry for copy up origin")
> Cc: <stable@vger.kernel.org> # v4.13
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  fs/overlayfs/namei.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> Miklos,
>
> This is a complimentary/fixup patch to the patch currently at the tip of
> overlayfs-next. I did not run into ENOENT in my tests, but it seems that
> all other places in overlayfs that call lookup_one_len_unlocked() check
> the ENOENT return value and treat it the same as negative dentry.
>
> I suppose this could be expected from some file systems?

I haven't done the research, but I suppose it's possible.  Nobody is
forcing filesystems to cache negative dentries, although not doing so
might be stupid.

However, the patch seems to be wrong, since it does not treat -ENOENT
quite the same way as a negative index (no warning for hard links).

Btw you can use "--suppress-cc=bodycc" to not spam stable@vger with
review patches.

Thanks,
Miklos

Re: [PATCH] ovl: handle ENOENT on index lookup

[Amir Goldstein]

On Fri, Oct 20, 2017 at 4:23 PM, Miklos Szeredi <miklos@szeredi.hu> wrote:
> On Fri, Oct 20, 2017 at 1:01 PM, Amir Goldstein <amir73il@gmail.com> wrote:
>> Fixes: 359f392ca53e ("ovl: lookup index entry for copy up origin")
>> Cc: <stable@vger.kernel.org> # v4.13
>> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
>> ---
>>  fs/overlayfs/namei.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> Miklos,
>>
>> This is a complimentary/fixup patch to the patch currently at the tip of
>> overlayfs-next. I did not run into ENOENT in my tests, but it seems that
>> all other places in overlayfs that call lookup_one_len_unlocked() check
>> the ENOENT return value and treat it the same as negative dentry.
>>
>> I suppose this could be expected from some file systems?
>
> I haven't done the research, but I suppose it's possible.  Nobody is
> forcing filesystems to cache negative dentries, although not doing so
> might be stupid.
>
> However, the patch seems to be wrong, since it does not treat -ENOENT
> quite the same way as a negative index (no warning for hard links).
>
> Btw you can use "--suppress-cc=bodycc" to not spam stable@vger with
> review patches.
>

OK, but Greg didn't seem annoyed by getting CC'ed - on the contrary..
Anyway, sent fixed v2 and CC'ed stable, since there was already a
discussion on v1

Amir.