On Tue, Sep 26, 2023 at 5:40 PM Mimi Zohar <zohar@xxxxxxxxxxxxx> wrote: > > On Thu, 2023-09-21 at 20:01 +0300, Amir Goldstein wrote: > > On Thu, Sep 21, 2023 at 7:31 PM Mimi Zohar <zohar@xxxxxxxxxxxxx> wrote: > > > ... > > > Let's see if Amir's patch actually fixes the original problem before > > > making any decisions. (Wishing for a reproducer of the original > > > problem.) > > > > > > > Confused. What is the "original problem"? > > I never claimed that my patch fixes the "original problem". > > I claimed [1] that my patch fixes a problem that existed before > > db1d1e8b9867, but db1d1e8b9867 added two more instances > > of that bug (wrong dereference of file->f_path). > > Apparently, db1d1e8b9867 introduced another bug. > > > > It looks like you should revert db1d1e8b9867, but regardless, > > I recommend that you apply my patch. My patch conflicts > > with the revert but the conflict is trivial - the two hunks that > > fix the new vfs_getattr_nosec() calls are irrelevant - the rest are. > > > > Thanks, > > Amir. > > > > [1] https://lore.kernel.org/linux-unionfs/20230913073755.3489676-1-amir73il@xxxxxxxxx/ > > Here are some of the issues with IMA/Overlay: > > 1. False positive syzbot IMA/overlay lockdep warnings. > 2, Not detecting file change with squashfs + overlay. > 3. Changes to the backing file are not detected by overlay (when > backing file is not in policy). > > Commit db1d1e8b9867 ("IMA: use vfs_getattr_nosec to get the i_version") > upstreamed to address 2, but has become unnecessary due to other > changes. According to Stefan, the problem subsequently was resolved > without either commit db1d1e8b9867 or 18b44bc5a672. (Kernel was not > bi-sected to find bug resolution.) > > Commit 18b44bc5a672 ("ovl: Always reevaluate the file signature for > IMA") to address 3. > > [PATCH] "ima: fix wrong dereferences of file->f_path" is probably > correct. Does it address any syzbot reports? > Not that I know of. Mimi, I am going to change my recommendation to - Please wait with applying my patch unless you know that it fixes a known bug, because: 1. I don't have a complete picture of ovl+IMA 2. I didn't find any specific test case to prove the bug 3. I have a plan to get rid of the file_real_path() anomaly Thanks, Amir.
