On Tue, May 15, 2018 at 4:23 PM, Vivek Goyal <vgoyal@xxxxxxxxxx> wrote: > On Tue, May 15, 2018 at 01:26:09PM +0300, Amir Goldstein wrote: >> Currently, there is a small window where ovl_obtain_alias() can >> race with ovl_instantiate() and create two different overlay inodes >> with the same underlying real non-dir non-hardlink inode. >> >> The race requires an adversary to guess the file handle of the >> yet to be created upper inode and decode the guessed file handle >> after ovl_creat_real(), but before ovl_instantiate(). >> >> This patch fixes the race, by using insert_inode_locked4() to add >> a newly created inode to icache. >> >> If the newly created inode apears to already exist in icache (hashed >> by the same real upper inode), we export this error to user instead >> of silently not hashing the new inode. > > So we might return an error to user saying operation failed, but still > create file on upper. Does that sound little odd? > Yes, but I don't see a better solution. > I am wondering why can't we call ovl_get_inode() in object creation > path. That should take care of race between creation path and file > handle decode and only one of the paths will get to instantiate and > initialize ovl_inode and other path will wait. > I don't even want to think if what you wrote makes sense. Remember that the use case we are talking about is quite imaginary. Ensuring internal structures consistency in our code and returning error to user is the right thing to do for imaginary use cases IMO. Thanks, Amir.
