linux-kernel - Re: [PATCH] add check do_direct

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:	Tue, 31 Jul 2007 15:59:45 -0700 (PDT)
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Andrew Morton <akpm@...ux-foundation.org>
cc:	Badari Pulavarty <pbadari@...ibm.com>,
	Joe Jin <joe.jin@...cle.com>,
	Zach Brown <zach.brown@...cle.com>,
	gurudas pai <gurudas.pai@...cle.com>, jens.axboe@...cle.com,
	lkml <linux-kernel@...r.kernel.org>, wen.gang.wang@...cle.com
Subject: Re: [PATCH] add check do_direct_IO() return val



On Tue, 31 Jul 2007, Andrew Morton wrote:
> 
> We zero out so many fields in there now that a kzalloc() might yield
> a net gain.  0.000001% in an unnamed benchmark!

Indeed. That's *especially* true since right now it passes a mostly 
totally uninitialized structure pointer down to direct_io_worker(), but it 
has actually initialized a _few_ fields. Ie the "die" pointer is almost 
totally filled with random crap, except for two members: dio->lock_type 
and dio->is_async have been initialized the rest is random crud.

That kind of mixing of totally-but-not-entirely-uninitialized pointer 
passing is just a recipe for disaster.

Does a patch like this work? I don't have any test-cases, but it would be 
good to have something like this tested and passed back with proper 
explanations and sign-offs.

		Linus

---
 fs/direct-io.c |   17 +----------------
 1 files changed, 1 insertions(+), 16 deletions(-)

diff --git a/fs/direct-io.c b/fs/direct-io.c
index 52bb263..901dc55 100644
--- a/fs/direct-io.c
+++ b/fs/direct-io.c
@@ -958,35 +958,22 @@ direct_io_worker(int rw, struct kiocb *iocb, struct inode *inode,
 	ssize_t ret2;
 	size_t bytes;
 
-	dio->bio = NULL;
 	dio->inode = inode;
 	dio->rw = rw;
 	dio->blkbits = blkbits;
 	dio->blkfactor = inode->i_blkbits - blkbits;
-	dio->start_zero_done = 0;
-	dio->size = 0;
 	dio->block_in_file = offset >> blkbits;
-	dio->blocks_available = 0;
-	dio->cur_page = NULL;
 
-	dio->boundary = 0;
-	dio->reap_counter = 0;
 	dio->get_block = get_block;
 	dio->end_io = end_io;
-	dio->map_bh.b_private = NULL;
 	dio->final_block_in_bio = -1;
 	dio->next_block_for_io = -1;
 
-	dio->page_errors = 0;
-	dio->io_error = 0;
-	dio->result = 0;
 	dio->iocb = iocb;
 	dio->i_size = i_size_read(inode);
 
 	spin_lock_init(&dio->bio_lock);
 	dio->refcount = 1;
-	dio->bio_list = NULL;
-	dio->waiter = NULL;
 
 	/*
 	 * In case of non-aligned buffers, we may need 2 more
@@ -994,8 +981,6 @@ direct_io_worker(int rw, struct kiocb *iocb, struct inode *inode,
 	 */
 	if (unlikely(dio->blkfactor))
 		dio->pages_in_io = 2;
-	else
-		dio->pages_in_io = 0;
 
 	for (seg = 0; seg < nr_segs; seg++) {
 		user_addr = (unsigned long)iov[seg].iov_base;
@@ -1183,7 +1168,7 @@ __blockdev_direct_IO(int rw, struct kiocb *iocb, struct inode *inode,
 		}
 	}
 
-	dio = kmalloc(sizeof(*dio), GFP_KERNEL);
+	dio = kzalloc(sizeof(*dio), GFP_KERNEL);
 	retval = -ENOMEM;
 	if (!dio)
 		goto out;

-
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/