linux-ext4 - Re: [GIT PULL] ext2, quota, and udf fixes for 6.6-rc1

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