linux-ext4 - Re: [PATCH 1/2] [PATCH] ext4: Add inode to the orphan list during block allocation failure

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:	Mon, 6 Apr 2009 12:05:09 +0200
From:	Jan Kara <jack@...e.cz>
To:	Theodore Tso <tytso@....edu>
Cc:	"Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com>,
	linux-ext4@...r.kernel.org, Jan Kara <jack@...e.cz>
Subject: Re: [PATCH 1/2] [PATCH] ext4: Add inode to the orphan list during
	block allocation failure

  Hi,

On Sat 04-04-09 23:11:16, Theodore Tso wrote:
> On Tue, Mar 31, 2009 at 02:59:25PM +0530, Aneesh Kumar K.V wrote:
> > We should add inode to the orphan list in the same transaction
> > as block allocation. This ensures that if we crash after a failed
> > block allocation and before we do a vmtruncate we don't leak block
> > (ie block marked as used in bitmap but not claimed by the inode).
> 
> How likely is this going to happen?  If it's a failure in the block
> allocation, we will have really end up with blocks outside i_size?
> And in that case, I'm missing how this ends up being a leaked block,
> since presumably the block is still being referenced by the inode.  Am
> I missing something here?
  Aneesh's changelog was not completely precise here. First note that some
problems (namely those in ext4_write_begin()) can happen only if blocksize
< pagesize - there we can succeed in allocating some blocks for the page
but not all of them. Since we then decide to redo the whole write, we have
to truncate blocks we have already allocated.
  The problem in ext4_..._write_end() is different (and rather easy to hit
under heavy memory pressure) - the problem is that generic_perform_write()
fails to copy data into our kernel page because the user page from which
we should copy the data has been paged-out. In this situation we again
decide to redo the write and so we should truncate the already allocated
blocks since i_size is still set to the value before write. Otherwise
we'd have blocks allocated beyond i_size and so a crash before we
successfully redo the write would "leak" blocks (you're right the inode
would be still referencing them but they'll be above i_size and I guess
it could confuse some code).

> I guess it can happen if do_journal_get_write_access() returns an
> error, which could potentially happen if there is a ENOMEM error
> coming from the jbd2 layer.  But that brings up another potential
> problem.  It's possible for vmtruncate() to fail; if ext4_truncate()
> fails too early (particularly in ext4_ext_truncate, due to a memory
> error in a jbd2 routines), it's possible for it to return without
> calling ext4_orphan_del() --- and stray inodes on the orphan list will
> cause the kernel to panic on umount.
> 
> I think this can be fixed by making sure that ext4_truncate() and
> ext4_ext_truncate() calls ext4_orphan_del() in *all* of their error
> paths.  That *should* the problem, since at the moment, it doesn't
> look vmtruncate() will return without calling inode->i_op->truncate().
> But could you double check this carefully?
  Ah, OK, that should be fixed. But note that current ext4_setattr()
does exactly the same thing on standard truncates - it adds inode to
orphan list and calls inode_setattr() which end's up calling vmtruncate().

								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