lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Fri, 20 Oct 2023 20:09:53 +0200
From:   Jan Kara <jack@...e.cz>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Andy Shevchenko <andriy.shevchenko@...el.com>,
        Baokun Li <libaokun1@...wei.com>,
        Josh Poimboeuf <jpoimboe@...nel.org>, Jan Kara <jack@...e.cz>,
        Nathan Chancellor <nathan@...nel.org>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        Kees Cook <keescook@...omium.org>,
        Ferry Toth <ftoth@...londelft.nl>,
        linux-fsdevel@...r.kernel.org, linux-ext4@...r.kernel.org
Subject: Re: [GIT PULL] ext2, quota, and udf fixes for 6.6-rc1

On Fri 20-10-23 10:26:26, Linus Torvalds wrote:
> On Fri, 20 Oct 2023 at 08:12, Andy Shevchenko
> <andriy.shevchenko@...el.com> wrote:
> >
> > > > --- a/fs/quota/dquot.c
> > > > +++ b/fs/quota/dquot.c
> > > > @@ -632,8 +632,10 @@ static inline int dquot_write_dquot(struct dquot *dquot)
> > > >  {
> > > >         int ret = dquot->dq_sb->dq_op->write_dquot(dquot);
> > > >         if (ret < 0) {
> > > > +#if 0
> > > >                 quota_error(dquot->dq_sb, "Can't write quota structure "
> > > >                             "(error %d). Quota may get out of sync!", ret);
> > > > +#endif
> > > >                 /* Clear dirty bit anyway to avoid infinite loop. */
> > > >                 clear_dquot_dirty(dquot);
> > > >         }
> >
> > Doing the same on the my branch based on top of v6.6-rc6 does not help.
> > So looks like a race condition somewhere happening related to that dirty bit
> > (as comment states it needs to be cleaned to avoid infinite loop, that's
> >  probably what happens).
> 
> Hmm. Normally, dirty bits should always be cleared *before* the
> write-back, not after it. Otherwise you might lose a dirty event that
> happened *during* writeback.

Yes, and normally we clear the dirty bit in dquot_commit() before writing
the dquot. However if there is an error in fs-private ->write_dquot()
helper before calling back into dquot_commit() (e.g. ext4 fails to start a
transaction), ->write_dquot() can return without clearing the dirty bit.
For dqput() to not loop indefinitely trying to clean the dquot, we clear
the dirty bit here just to be sure in case of error.

> But I don't know the quota code.
> 
> ... the fact that the #if 0 doesn't help in another case does say that
> it's not the quota_error() call itself. Which it really couldn't have
> been (apart from timing and compiler bugs), but it's still a data
> point, I guess.

Yeah, that's a bit weird. I'm really curious what the problem is.

								Honza
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR

Powered by blists - more mailing lists