On Thu, Nov 14, 2024 at 9:56 AM Amir Goldstein <amir73il@xxxxxxxxx> wrote: > > On Wed, Nov 13, 2024 at 8:30 PM Vinicius Costa Gomes > <vinicius.gomes@xxxxxxxxx> wrote: > > > > Amir Goldstein <amir73il@xxxxxxxxx> writes: > > > > > On Thu, Nov 7, 2024 at 1:57 AM Vinicius Costa Gomes > > > <vinicius.gomes@xxxxxxxxx> wrote: > > > > [...] > > > > > > > > Vinicius, > > > > > > While testing fanotify with LTP tests (some are using overlayfs), > > > kmemleak consistently reports the problems below. > > > > > > Can you see the bug, because I don't see it. > > > Maybe it is a false positive... > > > > Hm, if the leak wasn't there before and we didn't touch anything related to > > prepare_creds(), I think that points to the leak being real. > > > > But I see your point, still not seeing it. > > > > This code should be equivalent to the code we have now (just boot > > tested): > > > > ---- > > diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c > > index 136a2c7fb9e5..7ebc2fd3097a 100644 > > --- a/fs/overlayfs/dir.c > > +++ b/fs/overlayfs/dir.c > > @@ -576,8 +576,7 @@ static int ovl_setup_cred_for_create(struct dentry *dentry, struct inode *inode, > > * We must be called with creator creds already, otherwise we risk > > * leaking creds. > > */ > > - WARN_ON_ONCE(override_creds(override_cred) != ovl_creds(dentry->d_sb)); > > - put_cred(override_cred); > > + WARN_ON_ONCE(override_creds_light(override_cred) != ovl_creds(dentry->d_sb)); > > > > return 0; > > } > > ---- > > > > Does it change anything? (I wouldn't think so, just to try something) > > No, but I think this does: > > --- a/fs/overlayfs/dir.c > +++ b/fs/overlayfs/dir.c > @@ -576,7 +576,8 @@ static int ovl_setup_cred_for_create(struct dentry > *dentry, struct inode *inode, > * We must be called with creator creds already, otherwise we risk > * leaking creds. > */ > - WARN_ON_ONCE(override_creds(override_cred) != ovl_creds(dentry->d_sb)); > + old_cred = override_creds(override_cred); > + WARN_ON_ONCE(old_cred != ovl_creds(dentry->d_sb)); > put_cred(override_cred); > > return 0; > > Compiler optimized out override_creds(override_cred)? :-/ Nope, this voodoo did not help either. > > However, this is not enough. > > Dropping the ref of the new creds is going to drop the refcount to zero, > so that is incorrect, we need to return the reference to the new creds > explicitly to the callers. I will send a patch. And neither did this. The suspect memleak is still reported. Any other ideas? Thanks, Amir.
