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
| ||
|
Date: Fri, 27 Jul 2007 07:31:27 +0000 From: "Dave Young" <hidave.darkstar@...il.com> To: "Joe Jin" <joe.jin@...cle.com> Cc: "Andrew Morton" <akpm@...ux-foundation.org>, 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 >On 7/27/07, Joe Jin <joe.jin@...cle.com> wrote: > > 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? > Hi, If you are sure the problem is caused by EFAULT or ENOMEM , why not: if (ret) { dio_cleanup(dio); if (ret == -EFAULT || ret == -ENOMEM) goto out; break; } regards dave - 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/
Powered by blists - more mailing lists