linux-kernel - Re: [PATCH 4/7] O_NONBLOCK flag for readv2/preadv2

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: <x4961gnz844.fsf@segfault.boston.devel.redhat.com>
Date:	Tue, 16 Sep 2014 15:19:07 -0400
From:	Jeff Moyer <jmoyer@...hat.com>
To:	Milosz Tanski <milosz@...in.com>
Cc:	linux-kernel@...r.kernel.org,
	Christoph Hellwig <hch@...radead.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

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.

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?

Cheers,
Jeff
--
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/