linux-kernel - Re: [External] Re: [PATCH RESEND] ext4:fix different behavior of fsync when use fast commit

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] [day] [month] [year] [list]

Message-ID: <CAK896s7kJ47Q857u-+vBsera95tSJkxMy2Qhqk-91YjbR3QSdA@mail.gmail.com>
Date:   Fri, 7 Jan 2022 16:49:01 +0800
From:   Xin Yin <yinxin.x@...edance.com>
To:     "Theodore Ts'o" <tytso@....edu>
Cc:     harshadshirwadkar@...il.com, linux-ext4@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [External] Re: [PATCH RESEND] ext4:fix different behavior of
 fsync when use fast commit

Thanks for the explanation and direction , I will do another fix for this issue.



Best Regards,
Xin Yin


On Thu, Jan 6, 2022 at 11:22 AM Theodore Ts'o <tytso@....edu> wrote:
>
> On Fri, Dec 24, 2021 at 02:57:28PM +0800, Xin Yin wrote:
> > For the follow test example:
> > -mkdir test/
> > -create&write test/a.txt
> > -fsync test/a.txt
> > -crash (before a full commit)
> >
> > If fast commit is used then "a.txt" will lost, while the normal
> > journaling can recover it.
>
> The problem is that your proposed fix:
>
> > diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
> > index 3deb97b22ca4..4b843648ffe5 100644
> > --- a/fs/ext4/fast_commit.c
> > +++ b/fs/ext4/fast_commit.c
> > @@ -423,7 +423,7 @@ void __ext4_fc_track_create(handle_t *handle, struct inode *inode,
> >       args.op = EXT4_FC_TAG_CREAT;
> >
> >       ret = ext4_fc_track_template(handle, inode, __track_dentry_update,
> > -                                     (void *)&args, 0);
> > +                                     (void *)&args, 1);
> >       trace_ext4_fc_track_create(inode, dentry, ret);
> >  }
>
> affects both file creations as well as directory creations (mkdir).
> Putting the inode on the fast commit list is something that is meant
> for files, and means that on a fast commit we need to force the data
> blocks out.  So it seems that isn't the right fix for the problem.
>
> Why do something really simple?  Look at the parent directory's inode,
> and check its i_sync_tid.  If it's not equal to
> handle->h_transaction->t_tid, then it's safe to do the fast commit.
> If it's equal to the current transaction, we can either force a full
> commit.
>
> Optionally, in the case where i_sync_tid == current tid, since there's
> a chance that the parent directory's inode could have been freshly
> fetched from disk (see __ext4_iget() in fs/ext4/inode.c), we could
> compare its i_crtime against ktime_get_real_seconds(), and if the
> inode was created in the last 2*journal->j_commit_interval/HZ seconds,
> it's safe to do a fast commit; otherwise do a full commit.
>
> Cheers,
>
>                                         - Ted