diff mbox

[3/3] ovl: redirect on rename-dir

Message ID CAOQ4uxgK9+Nwxtj9GCHp-fFg7Gsu0HMA2-MeguBJ=BWxQATWmw@mail.gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Amir Goldstein Nov. 4, 2016, 9:29 a.m. UTC 
On Thu, Nov 3, 2016 at 5:50 PM, Miklos Szeredi <miklos@szeredi.hu> wrote:
> On Fri, Oct 28, 2016 at 6:15 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
>> On Tue, Oct 25, 2016 at 09:34:47AM +0200, Miklos Szeredi wrote:
...
>>
>> I'm not sure if vfs_path_lookup() is the right tool here.  It might be
>> usable for making such a tool, but as it is you are setting one hell of
>> a trap for yourself...
>
> Agreed, it's not the right tool.   A custom loop of lookup_one_len's
> should work much better and doesn't add all that much complexity.
> Updated patch pushed to:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/vfs.git #redirect
>
> This version also passes the recycling tests by Amir and enables the
> redirect feature by default on an empty upperdir.
>

Miklos,

You did not address my comment about the 'stack' allocation overflow
in ovl_lookup
I believe the (possible) overflow is demonstrated by the following debug patch:


@@ -238,6 +240,8 @@ struct dentry *ovl_lookup(struct inode *dir,
struct dentry *dentry,
                                                break;
                                if (WARN_ON(i == poe->numlower))
                                        break;
+                               if (WARN_ON(poe->numlower - i - 1 > stackroom))
+                                       break;
                        }
                }
        }

Comments

Miklos Szeredi Nov. 4, 2016, 1:48 p.m. UTC | #1 
On Fri, Nov 4, 2016 at 10:29 AM, Amir Goldstein <amir73il@gmail.com> wrote:

> You did not address my comment about the 'stack' allocation overflow
> in ovl_lookup
> I believe the (possible) overflow is demonstrated by the following debug patch:

Oops, missed that.  Good spotting!

And there's more shit that unionfs-testsuite didn't discover (not even
involving multiple layers):

rm -rf /lower /upper /work
mkdir -p /lower/a/b/c /upper /work
mount -t overlay overlay -oupperdir=/upper,lowerdir=/lower,workdir=/work /mnt
mv /mnt/a /mnt/z
mv /mnt/z/b /mnt/q
ls /mnt/q
umount /mnt
mount -t overlay overlay -oupperdir=/upper,lowerdir=/lower,workdir=/work /mnt
ls /mnt/q
umount /mnt

Next update coming up...

Thanks,
Miklos
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
index c7cacbb..7171bfb 100644
--- a/fs/overlayfs/namei.c
+++ b/fs/overlayfs/namei.c
@@ -231,5 +231,7 @@  struct dentry *ovl_lookup(struct inode *dir,
struct dentry *dentry,
                                goto out_put;

                        if (redirect && poe != dentry->d_sb->s_root->d_fsdata) {
+                               int stackroom = poe->numlower - ctr;
+
                                poe = dentry->d_sb->s_root->d_fsdata;