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]

Message-ID: <20070727071547.GA25084@joejin-pc.cn.oracle.com>
Date:	Fri, 27 Jul 2007 15:15:47 +0800
From:	Joe Jin <joe.jin@...cle.com>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	Joe Jin <joe.jin@...cle.com>, torvalds@...ux-foundation.org,
	jens.axboe@...cle.com, linux-kernel@...r.kernel.org,
	gurudas.pai@...cle.com, wen.gang.wang@...cle.com,
	Badari Pulavarty <pbadari@...ibm.com>,
	Zach Brown <zach.brown@...cle.com>
Subject: Re: [PATCH] add check do_direct_IO() return val

> I think we still want to run dio_cleanup() if do_direct_IO() failed? 
> Otherwise we can leak pages.
> 
> And there's nothing special about EFAULT or ENOMEM here: if do_direct_IO()
> returns any error then that's it: we bale out, yes?
> 

Yes, I think we'll out from here if get EFAULT/ENOMEM error, also maybe -EIO
return, return diretly should ok.

> In fact I'm suspecting that this is what the code in there used to do. 
> Something like:
> 
> 	for (...) {
> 		...
> 		ret = do_direct_IO(...);
> 		...
> 		if (ret) {
> 			dio_dleanup(dio);
> 			break
> 		}
> 	}
> 	return ret;
> 

Yes, we need call dio_cleanup() to release page cache, I lost it.

However, we need do more while return -ENOTBLK, right?
so I think the patch maybe like following:


--- linux-2.6.22/fs/direct-io.c.orig	2007-07-27 14:39:15.000000000 +0800
+++ linux-2.6.22/fs/direct-io.c	2007-07-27 15:08:58.000000000 +0800
@@ -1032,18 +1032,19 @@ direct_io_worker(int rw, struct kiocb *i
 					blkbits);
 
 		if (ret) {
+			if (ret == -ENOTBLK && (rw & WRITE)) {
+				/*
+				 * The remaining part of the request will be
+				 * be handled by buffered I/O when we return
+				 */
+				ret = 0;
+				break;
+			}
 			dio_cleanup(dio);
-			break;
+			goto out;
 		}
 	} /* end iovec loop */
 
-	if (ret == -ENOTBLK && (rw & WRITE)) {
-		/*
-		 * The remaining part of the request will be
-		 * be handled by buffered I/O when we return
-		 */
-		ret = 0;
-	}
 	/*
 	 * There may be some unwritten disk at the end of a part-written
 	 * fs-block-sized block.  Go zero that now.




> _
> 
> However I'd like to ask you guys to carefully review and test that please.
> 
Gurudas, would you please give more test of this patch?

Thanks, 
Joe
-
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/