linux-kernel - Re: [PATCH v3] UDF: Add support for O

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: <20120905125533.GD18051@quack.suse.cz>
Date:	Wed, 5 Sep 2012 14:55:33 +0200
From:	Jan Kara <jack@...e.cz>
To:	Ian Abbott <abbotti@....co.uk>
Cc:	Jan Kara <jack@...e.cz>, Ian Abbott <ian.abbott@....co.uk>,
	lkml <linux-kernel@...r.kernel.org>,
	"linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>
Subject: Re: [PATCH v3] UDF: Add support for O_DIRECT

On Wed 05-09-12 14:05:20, Jan Kara wrote:
> On Tue 04-09-12 16:11:32, Ian Abbott wrote:
> > >>1. Small files stored in the ICB (inode control block?): just return 0
> > >>from the new udf_adinicb_direct_IO() handler to fall back to buffered
> > >>I/O.  For direct writes, there is a "gotcha" to deal with when
> > >>generic_file_direct_write() in mm/filemap.c invalidates the pages.  In
> > >>the udf_adinicb_writepage() handler, only part of the page data will be
> > >>valid and the rest will be zeroed out, so only copy the valid part into
> > >>the ICB.  (This is actually a bit inefficient as udf_adinicb_write_end()
> > >>will have already copied the data into the ICB once, but it's pretty
> > >>likely that the file will grow to the point where its data can no longer
> > >>be stored in the ICB and will be moved to a different area of the file
> > >>system.  At that point, a different direct_IO handler will be used - see
> > >>below.)
> > >   Sorry, I didn't quite get this. What is the problem with copying all the
> > >data to inode in udf_adinicb_writepage() as it is now?
> > 
> > Part of the good data in the ICB outside the range being addressed
> > would get overwritten by zeroes.  This can be tested by creating a
> > UDF filesystem with 4KiB blocks and with small files stored in the
> > ICB, backed by a block device with 512 byte sectors.  Create a 2KiB
> > file with random (or non-zero) data on the file system so that its
> > data gets stored in the ICB.  Then open the file for writing without
> > truncation and with the O_DIRECT flag set, write 512 bytes at some
> > 512 byte offset within the 2KiB file and close it.  If you then
> > hexdump the file, you'll find some of the old random data has been
> > zeroed out.
>   But don't you fall back to buffered IO for files in ICB? So then no
> zeroing should happen?
  Oh, I've tested things now and the bug is in buffered write as well!
It has nothing to do with direct IO. We cannot use simple_write_begin() for
UDF when the file is in ICB. I'll write a proper fix.

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