| 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 18:07:13 +0530
From: gurudas pai <gurudas.pai@...cle.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, 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
Joe Jin 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?
I tested Andrew's patch and panic was gone but got few ENOTBLK.
So I tried with Joe's patch , both panic and ENOTBLK are gone now.
But in Joe's patch if (ret == -ENOTBLK && (rw & WRITE)), dio_cleanup(dio)
was not getting called because of break. So I moved dio_cleanup just
after if (ret).
I tested with this patch, both panic and ENOTBLK are gone now.
Here is the modified patch:
--- linux-2.6.22/fs/direct-io.c.orig 2007-07-27 03:06:39.000000000 -0700
+++ linux-2.6.22/fs/direct-io.c 2007-07-27 03:24:55.000000000 -0700
@@ -1033,17 +1033,18 @@
if (ret) {
dio_cleanup(dio);
- break;
+ 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;
+ }
+ 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.
@@ -1112,7 +1113,7 @@
kfree(dio);
} else
BUG_ON(ret != -EIOCBQUEUED);
-
+out:
return ret;
}
-
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