linux-kernel - Re: [PATCH] vfs: introduce UMOUNT_WAIT which waits for umount completion

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: <20170914011048.GA47448@jaegeuk-macbookpro.roam.corp.google.com>
Date:   Wed, 13 Sep 2017 18:10:48 -0700
From:   Jaegeuk Kim <jaegeuk@...nel.org>
To:     Al Viro <viro@...IV.linux.org.uk>
Cc:     linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
        linux-f2fs-devel@...ts.sourceforge.net
Subject: Re: [PATCH] vfs: introduce UMOUNT_WAIT which waits for umount
 completion

On 09/14, Al Viro wrote:
> On Wed, Sep 13, 2017 at 04:31:16PM -0700, Jaegeuk Kim wrote:
> > Hi Al,
> > 
> > On 09/14, Al Viro wrote:
> > > On Wed, Sep 13, 2017 at 01:09:41PM -0700, Jaegeuk Kim wrote:
> > > > +	if (!retval && (flags & UMOUNT_WAIT)) {
> > > > +		if (likely(!(current->flags & PF_KTHREAD)))
> > > > +			task_work_run();
> > > 
> > > This is complete crap.  The same damn thing will be done by
> > > caller of sys_umount() pretty much immediately afterwards.
> > > I'm not sure what it is that you are trying to paper over,
> > > but this is just plain wrong.
> > 
> > Okay.
> > 
> > > What _is_ the semantics of UMOUNT_WAIT?  What does it guarantee,
> > > and what would be supplying it to umount(2)?
> > 
> > When android tries to reboot the system, it calls umount(2) without any flag.
> > Then, mntput_no_expire() will add delayed_mntput_work() which finally does
> > cleanup_mnt() later. In the mean time, android proceeded to shutdown all
> > the UFS devices.
> 
> Why has task_work_add() failed?  Or is that umount(2) issued by a kernel thread?

Android triggers umount(2) by init process, which is definitely not a kernel
thread. But, we've seen some kernel panics which say umount(2) was succeeded,
but ext4 triggered a kernel panic due to EIO after then like below. I'm also
not sure task_work_run() would be also safe enoughly. May I ask where I can
find sys_umount() calls task_work_run()?

[254012.860565] c4  12426 [<ffffff909b6a7ebc>] panic+0x184/0x37c
[254012.860589] c4  12426 [<ffffff909b88e724>] __ext4_abort+0x198/0x19c
[254012.860606] c4  12426 [<ffffff909b897468>] ext4_put_super+0x80/0x2b4
[254012.860629] c4  12426 [<ffffff909b7e7274>] generic_shutdown_super+0x68/0xd0
[254012.860646] c4  12426 [<ffffff909b7e87fc>] kill_block_super+0x1c/0x5c
[254012.860663] c4  12426 [<ffffff909b7e70dc>] deactivate_locked_super+0x5c/0xc0
[254012.860679] c4  12426 [<ffffff909b7e71a8>] deactivate_super+0x68/0x74
[254012.860696] c4  12426 [<ffffff909b80a25c>] cleanup_mnt+0xb0/0x12c
[254012.860712] c4  12426 [<ffffff909b80a310>] delayed_mntput+0x38/0x4c
[254012.860737] c4  12426 [<ffffff909b6c6524>] process_one_work+0x1e0/0x490
[254012.860753] c4  12426 [<ffffff909b6c6094>] worker_thread+0x314/0x494
[254012.860771] c4  12426 [<ffffff909b6cb35c>] kthread+0xdc/0xec
[254012.860790] c4  12426 [<ffffff909b683860>] ret_from_fork+0x10/0x30