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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 16 Sep 2014 15:44:13 -0400 From: Milosz Tanski <milosz@...in.com> To: Jeff Moyer <jmoyer@...hat.com> Cc: LKML <linux-kernel@...r.kernel.org>, Christoph Hellwig <hch@...radead.org>, "linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>, linux-aio@...ck.org, Mel Gorman <mgorman@...e.de>, Volker Lendecke <Volker.Lendecke@...net.de>, Tejun Heo <tj@...nel.org> Subject: Re: [PATCH 4/7] O_NONBLOCK flag for readv2/preadv2 On Tue, Sep 16, 2014 at 3:19 PM, Jeff Moyer <jmoyer@...hat.com> wrote: > Milosz Tanski <milosz@...in.com> writes: > >> Filesystems that generic_file_read_iter will not be allowed to perform >> non-blocking reads. This only will read data if it's in the page cache and if >> there is no page error (causing a re-read). >> >> Signed-off-by: Milosz Tanski <milosz@...in.com> > >> @@ -1662,6 +1676,10 @@ no_cached_page: >> goto out; >> } >> goto readpage; >> + >> +would_block: >> + error = -EAGAIN; >> + goto out; >> } > > Why did you put the wouldblock label inside the loop? That should be > pushed down to just above out, and then you can get rid of the goto. When I put the code outside the loop it actually looked worse (imo): } goto out; would_block: error = -EAGAIN; out: ... > > Other than that, it looks like you put the check in all the right places > in that function. > >> out: >> @@ -1697,6 +1715,9 @@ generic_file_read_iter(struct kiocb *iocb, struct iov_iter *iter, int flags) >> size_t count = iov_iter_count(iter); >> loff_t size; >> >> + if (flags & O_NONBLOCK) >> + return -EAGAIN; >> + > > If a program is attempting non-blocking reads on a file opened with > O_DIRECT, I think returning -EAGAIN is very misleading. Better to > return -EINVAL in this case, and maybe check that earlier in the stack? Point taken and I can fix this for the next version further up the stack. A longer term question is how the flags the file is open with interact with the read/write flags ... since I imagine folks will want to add other flags (like force a SYNC write). > > Cheers, > Jeff -- Milosz Tanski CTO 16 East 34th Street, 15th floor New York, NY 10016 p: 646-253-9055 e: milosz@...in.com -- 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