All of lore.kernel.org
 help / color / mirror / Atom feed
From: Theodore Ts'o <tytso@mit.edu>
To: Vegard Nossum <vegard.nossum@oracle.com>
Cc: linux-ext4@vger.kernel.org, Michael Halcrow <mhalcrow@google.com>,
	Ildar Muslukhov <ildarm@google.com>,
	Jaegeuk Kim <jaegeuk@kernel.org>
Subject: Re: kernel BUG at fs/ext4/inode.c:3709! (Re: open bugs found by fuzzing)
Date: Fri, 15 Jul 2016 15:49:33 -0400	[thread overview]
Message-ID: <20160715194933.GE26465@thunk.org> (raw)
In-Reply-To: <578923EF.1020305@oracle.com>

On Fri, Jul 15, 2016 at 07:57:03PM +0200, Vegard Nossum wrote:
> I thought all the inodes on the orphan list were completely unreachable,
> but that's obviously not true following your explanations (thanks!) and
> another peek at the cleanup function which only does the truncate in the
> first place if ->i_nlink is non-zero, I missed that earlier. I guess
> this comment confused me:
> 
> /* ext4_orphan_cleanup() walks a singly-linked list of inodes (starting at
>  * the superblock) which were deleted from all directories, but held open by
>  * a process at the time of a crash.

Yeah, that comment is **badly** (as well over a decade) out-of-date.
Sorry.  :-/

The problem is fixing this is messy.  The problem is if we just set
i_size without zeroing out the bytes between i_size and the end of the
page, we're asking for trouble.  For example, you could think that
it's enough to have ext4_readpage() care of doing the zeroing after
the block is read from disk and then decrypted, but what if the user
does a sparse write beyond i_size?

So either we allow truncate(2) to be non-atomic with respect to
crashes for encrypted files, or alternatively we would have to set a
flag in the inode indicating that the bytes between i_size and the end
of the page must be zeroed before the file can be modified in any way,
or before an attempted O_DIRECT read of the last block, and then when
we read the inode from diks into the inode cache, if the bit is set
and we have the encryption key (which we would need if we want to
modify the file), we could take zeroing the tail end of the file then.

Yulch.   Definitely a bit of a hack.  :-(

					- Ted

P.S. Fortunately, this is a very rare case in practice, so I doubt we
would have hit it in a while, but it's definitely something we need to
fix, one way or another.

  reply	other threads:[~2016-07-15 19:49 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-14 21:10 open bugs found by fuzzing Vegard Nossum
2016-07-15 13:39 ` kernel BUG at fs/ext4/inode.c:3709! (Re: open bugs found by fuzzing) Vegard Nossum
2016-07-15 17:24   ` Theodore Ts'o
2016-07-15 17:57     ` Vegard Nossum
2016-07-15 19:49       ` Theodore Ts'o [this message]
2016-07-16 16:15         ` Vegard Nossum

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160715194933.GE26465@thunk.org \
    --to=tytso@mit.edu \
    --cc=ildarm@google.com \
    --cc=jaegeuk@kernel.org \
    --cc=linux-ext4@vger.kernel.org \
    --cc=mhalcrow@google.com \
    --cc=vegard.nossum@oracle.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.