On Fri, Oct 5, 2018 at 1:34 PM Mimi Zohar <zohar@xxxxxxxxxxxxx> wrote: > > On Thu, 2018-10-04 at 21:57 -0500, Goldwyn Rodrigues wrote: > > On 11:52 04/10, Mimi Zohar wrote: > > > On Thu, 2018-10-04 at 00:35 +0200, Miklos Szeredi wrote: > > > > > > > Right, if it's done from last fput() then it's at least not a security hole. > > > > > > > > This hack may work for some filesystems, but as you noticed, it won't > > > > work for overlayfs. And if probably won't work for a number of other > > > > filesystems as well: the fs can assume that f_mode & FMODE_READ will > > > > remain off if it was off at open time. > > > > > > > > The proper way to handle it generally is to open a separate instance > > > > of the same file from IMA with O_RDONLY and use that to calculate the > > > > hash. There's really no point in reusing the same file and hacking > > > > the f_mode bits. > > > > > > Is there an appropriate low level kernel call for creating a new file > > > descriptor that would be appropriate? dentry_open(), like the call in > > > file_clone_open(), has a lot of overhead, including the LSM calls. > > > Calculating the file hash always needs to work. > > > > > > > Mimi, > > > > I have formulated a patch which is working for me on overlayfs. I am > > using dentry_open(), which I agree may have overhead. While this > > opens up the possibility of using it for files opened with O_DIRECT > > which may end up getting the file into pagecache. > > > > Subject: [PATCH] Open new file instance O_RDONLY to calculate hash > > > > Using the open struct file to calculate the hash does not work > > with overlayfs, because the real struct file is hidden behind the > > overlays struct file. So, any file->f_mode manipulations are not > > reflected on the real struct file. So, open the file again, read and > > calculate the hash. > > > > As a byproduct, we can remove all instance of f_mode manipulations > > and can work with O_DIRECT as well. > > > > Signed-off-by: Goldwyn Rodrigues <rgoldwyn@xxxxxxxx> > > By "overhead", I didn't mean it from a performance perspective, but > was concerned about the dentry_open() failing. If "dentry_open" fails > for any reason, the file hash will not be re-calculated, causing > future file opens to fail. open_with_fake_path() looks like a better fit here. Skips irrelevant security checks and less likely to fail. Thanks, Amir.
