On Wed, May 16, 2018 at 11:51 AM, Amir Goldstein amir73il@gmail.com wrote:
On Wed, May 16, 2018 at 11:34 AM, Miklos Szeredi miklos@szeredi.hu wrote:
On Tue, May 15, 2018 at 3:37 PM, Amir Goldstein amir73il@gmail.com wrote:
On Tue, May 15, 2018 at 4:23 PM, Vivek Goyal vgoyal@redhat.com 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.
Might be better to kick the other, offending inode out, instead of returning an error. It would also simplify the error handling.
We can do that by creating an ovl_inode_test_kick() variant that unhashes the inode on match. Also needs insert_inode_locked4() to use hlist_for_each_entry_safe() instead of hlist_for_each_entry().
Do you really think that this corner use case calls for such actions, as creating flavors of inode cache helpers?
Yes, if it simplifies error handling.
Remember that the so called "offending" inode, is not offending in a way that is wrong or incomplete in any way.
Right, so what about just using that inode instead of erroring out?
Thanks, Miklos