linux-kernel - Re: [RFC v2 4/5] 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]

Date:	Mon, 22 Sep 2014 13:12:35 -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>, "Theodore Ts'o" <tytso@....edu>,
	Al Viro <viro@...iv.linux.org.uk>
Subject: Re: [RFC v2 4/5] O_NONBLOCK flag for readv2/preadv2

Milosz Tanski <milosz@...in.com> writes:

> diff --git a/fs/read_write.c b/fs/read_write.c
> index 3db2e87..29b5823 100644
> --- a/fs/read_write.c
> +++ b/fs/read_write.c
> @@ -864,8 +864,10 @@ ssize_t vfs_readv(struct file *file, const struct iovec __user *vec,
>  		return -EBADF;
>  	if (!(file->f_mode & FMODE_CAN_READ))
>  		return -EINVAL;
> -	if (flags & ~0)
> +	if (flags & ~RWF_NONBLOCK)
>  		return -EINVAL;
> +	if ((file->f_flags & O_DIRECT) && (flags & RWF_NONBLOCK))
> +		return -EAGAIN;

Just to close out our discussion on EINVAL for O_DIRECT with
RWF_NONBLOCK:

After discussing this with Zach, I agree that EAGAIN would be better.
There may be libraries that lack context and may benefit from the EAGAIN
return value.

> +/* These flags are used for the readv/writev syscalls with flags. */
> +#define RWF_NONBLOCK O_NONBLOCK

I'm not sure this make sense.  As I mentioned earlier, the epoll variant
made sense because the flag was shared (passed unmodified to other vfs
calls that understand it).  Here we can just define an entirely new flag
space.  Unless, of course, someone can come up with a reason why this
/would/ make sense?

> diff --git a/mm/filemap.c b/mm/filemap.c
> index e0919ba..6b7aba8 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -1483,7 +1483,10 @@ static ssize_t do_generic_file_read(struct file *filp, loff_t *ppos,
>  		cond_resched();
>  find_page:
>  		page = find_get_page(mapping, index);
> +

Please resist the urge to add whitespace.

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/