linux-ext4 - Re: quota: dqio

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]

Message-ID: <8209641.NLfoyJy6gT@panda>
Date:   Thu, 03 Aug 2017 16:55:40 +0300
From:   Andrew Perepechko <anserper@...dex.ru>
To:     Wang Shilong <wangshilong1991@...il.com>
Cc:     Shuichi Ihara <sihara@....com>, Wang Shilong <wshilong@....com>,
        Li Xi <lixi@....com>,
        Ext4 Developers List <linux-ext4@...r.kernel.org>,
        Jan Kara <jack@...e.cz>, linux-fsdevel@...r.kernel.org
Subject: Re: quota: dqio_mutex design

Let me put it this way:

Under file creation from different threads, ext4 will generate a series of
dquot updates (incore and then ondisk, through journal):

dquot update1
dquot update2
dquot update3
...
dquot updateN

Either with my patch or without it, ondisk dquot update through journal
may miss dquot update1, dquot update2, ... dquot update{N-1}.

You can easily see that from the code of dquot_commit():

int dquot_commit(struct dquot *dquot)
{
        int ret = 0;
        struct quota_info *dqopt = sb_dqopt(dquot->dq_sb);

        mutex_lock(&dqopt->dqio_mutex);
        spin_lock(&dq_list_lock);
        if (!clear_dquot_dirty(dquot)) {
                spin_unlock(&dq_list_lock);
                goto out_sem;
        }
...
}


If actual dquot_commit() wrote dquot update N, the threads commiting
updates 1 through N-1 will exit immediately once they get dqio_mutex
since the dquot will NOT be dirty.

My patch only avoids blocking on dqio_mutex when we know for sure
that another will NECESSARILY write the needed or a FRESHER dquot ondisk.

> > This change mean if this dquot is dirty we skip, this
> > won't work because in this way, quota update is only kept in vfs dquota
> > memory and newer update is not wrote to journal file and not wrapped into
> > transaction too.
> 
> That's not true.
> 
> As I explained earlier, having DQ_MOD_B set at this point means another
> thread is going to write dquot but hasn't yet started doing so. This thread
> does not care whether it updates the ondisk dquot with its own data or with
> fresher data which came from another thread. In-core dquot has no indication
> of whose data in contains.
> 
> As I also explained earlier, the update cannot happen in the context of
> another transaction because thread A which sees DQ_MOD_B set and thread
> B which is running dquot_commit() both have journal handles to the same
> transaction. There's only one running transaction at a time and thread B
> does not switch to another transaction.
> 
> Please read the code carefully.
> 
> > This is not what journal quota means to do.
> > 
> > 
> > Thanks,
> > Shilong
> > 
> > > Thank you,
> > > Andrew