linux-ext4 - Re: [PATCH 03/10] ext4: fix unwritten counter leakage

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:	Thu, 27 Sep 2012 15:07:18 +0200
From:	Jan Kara <jack@...e.cz>
To:	Dmitry Monakhov <dmonakhov@...nvz.org>
Cc:	Jan Kara <jack@...e.cz>, linux-ext4@...r.kernel.org, tytso@....edu,
	lczerner@...hat.com
Subject: Re: [PATCH 03/10] ext4: fix unwritten counter leakage

On Thu 27-09-12 16:54:13, Dmitry Monakhov wrote:
> On Thu, 27 Sep 2012 14:34:20 +0200, Jan Kara <jack@...e.cz> wrote:
> > On Thu 27-09-12 16:19:01, Dmitry Monakhov wrote:
> > > On Wed, 26 Sep 2012 15:07:14 +0200, Jan Kara <jack@...e.cz> wrote:
> > > > On Mon 24-09-12 15:44:13, Dmitry Monakhov wrote:
> > > > > ext4_set_io_unwritten_flag() will increment i_unwritten counter, so
> > > > > once we mark end_io with END_IO_UNWRITTEN we have to revert it back
> > > >                            ^^ EXT4_IO_END_UNWRITTEN
> > > > > on error path.
> > > > > 
> > > > >  - add missed error checks to prevent counter leakage
> > > > >  - ext4_end_io_nolock() will clear END_IO_UNWRITTEN flag to signal
> > > >                                      ^^ EXT4_IO_END_UNWRITTEN
> > > > >    that conversion finished.
> > > > >  - add BUGON to free_end_io() to prevent similar leackage in future.
> > > >          ^^ BUG_ON    ^^ext4_free_io_end()         ^^ leakage
> > > > 
> > > > > Visiable effect of this bug is that unaligned aio_stress may deadlock
> > > >   ^^ Visible
> > > > 
> > > >   Umm, and won't it be more foolproof it we just decrement i_unwritten in
> > > > ext4_free_io_end() when we see EXT4_IO_END_UNWRITTEN set?
> > > I'd like to consider BUG_ON inside ext4_free_io_end as a sanity check to
> > > force all callers to perform all necessary error checks in known context. 
> >   I'm not sure how "performing all necessary error checks in known context"
> > relates to ext4_free_io_end() cleaning up the structure on its own or
> > whether someone has to do it beforehand... Can you maybe elaborate a bit
> > more?
> I assume that if end_io was tagged with UNWRITTEN flag it should goes trough
> complete_io_list and end_io_nolock(), or caller should cancel it by
> itself in case of error, otherwise we may miss valid unwritten end_io
> but was not scheduled to complete_end_io routine by occasion (and endup
> in silent data loss). In my opinion at the time then ext4_free_io_end() 
> was called all possible conversions should be completed.
  Fair enough. I just hated verifying all the paths in extent.c and
checking whether they could happen to have end_io with UNWRITTEN flag set.

								Honza
-- 
Jan Kara <jack@...e.cz>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html