| 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
| ||
|
Message-ID: <5220FB1B.1060304@oracle.com>
Date: Fri, 30 Aug 2013 15:05:47 -0500
From: Dave Kleikamp <dave.kleikamp@...cle.com>
To: Benjamin LaHaise <bcrl@...ck.org>
CC: linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
Andrew Morton <akpm@...ux-foundation.org>,
"Maxim V. Patlasov" <mpatlasov@...allels.com>,
Zach Brown <zab@...bo.net>
Subject: Re: [PATCH V8 15/33] aio: add aio support for iov_iter arguments
Sorry for the lack of response. Getting back to this.
On 08/21/2013 08:55 AM, Benjamin LaHaise wrote:
> On Thu, Jul 25, 2013 at 12:50:41PM -0500, Dave Kleikamp wrote:
>> This adds iocb cmds which specify that memory is held in iov_iter
>> structures. This lets kernel callers specify memory that can be
>> expressed in an iov_iter, which includes pages in bio_vec arrays.
>>
>> Only kernel callers can provide an iov_iter so it doesn't make a lot of
>> sense to expose the IOCB_CMD values for this as part of the user space
>> ABI.
>
> I don't think adding the IOCB_CMD_{READ,WRITE}_ITER operations to
> include/uapi/linux/aio_abi.h is the right thing to do here -- they're
> never going to be used by userland, and care certainly not part of the
> abi we're presenting to userland. I'd suggest moving these opcodes to
> include/linux/aio.h.
Agreed.
> Also, if you make the values > 16 bits, userland
> will never be able to pass them in inadvertently (although things look
> okay if that does happen at present).
I'd have to change the declaration of ki_opcode to an int. This
shouldn't be a problem since it'll be padded to a long anyway.
>
> -ben
>
>> But kernel callers should also be able to perform the usual aio
>> operations which suggests using the the existing operation namespace and
>> support code.
>>
>> Signed-off-by: Dave Kleikamp <dave.kleikamp@...cle.com>
>> Cc: Zach Brown <zab@...bo.net>
>> ---
>> fs/aio.c | 67 ++++++++++++++++++++++++++++++++++++++++++++
>> include/linux/aio.h | 3 ++
>> include/uapi/linux/aio_abi.h | 2 ++
>> 3 files changed, 72 insertions(+)
>>
>> diff --git a/fs/aio.c b/fs/aio.c
>> index c65ba13..0da82c0 100644
>> --- a/fs/aio.c
>> +++ b/fs/aio.c
>> @@ -991,6 +991,48 @@ static ssize_t aio_setup_single_vector(int rw, struct kiocb *kiocb)
>> return 0;
>> }
>>
>> +static ssize_t aio_read_iter(struct kiocb *iocb)
>> +{
>> + struct file *file = iocb->ki_filp;
>> + ssize_t ret;
>> +
>> + if (unlikely(!is_kernel_kiocb(iocb)))
>> + return -EINVAL;
>> +
>> + if (unlikely(!(file->f_mode & FMODE_READ)))
>> + return -EBADF;
>> +
>> + ret = security_file_permission(file, MAY_READ);
>> + if (unlikely(ret))
>> + return ret;
>> +
>> + if (!file->f_op->read_iter)
>> + return -EINVAL;
>> +
>> + return file->f_op->read_iter(iocb, iocb->ki_iter, iocb->ki_pos);
>> +}
>> +
>> +static ssize_t aio_write_iter(struct kiocb *iocb)
>> +{
>> + struct file *file = iocb->ki_filp;
>> + ssize_t ret;
>> +
>> + if (unlikely(!is_kernel_kiocb(iocb)))
>> + return -EINVAL;
>> +
>> + if (unlikely(!(file->f_mode & FMODE_WRITE)))
>> + return -EBADF;
>> +
>> + ret = security_file_permission(file, MAY_WRITE);
>> + if (unlikely(ret))
>> + return ret;
>> +
>> + if (!file->f_op->write_iter)
>> + return -EINVAL;
>> +
>> + return file->f_op->write_iter(iocb, iocb->ki_iter, iocb->ki_pos);
>> +}
>> +
>> /*
>> * aio_setup_iocb:
>> * Performs the initial checks and aio retry method
>> @@ -1042,6 +1084,14 @@ rw_common:
>> ret = aio_rw_vect_retry(req, rw, rw_op);
>> break;
>>
>> + case IOCB_CMD_READ_ITER:
>> + ret = aio_read_iter(req);
>> + break;
>> +
>> + case IOCB_CMD_WRITE_ITER:
>> + ret = aio_write_iter(req);
>> + break;
>> +
>> case IOCB_CMD_FDSYNC:
>> if (!file->f_op->aio_fsync)
>> return -EINVAL;
>> @@ -1116,6 +1166,23 @@ void aio_kernel_init_rw(struct kiocb *iocb, struct file *filp,
>> }
>> EXPORT_SYMBOL_GPL(aio_kernel_init_rw);
>>
>> +/*
>> + * The iter count must be set before calling here. Some filesystems uses
>> + * iocb->ki_left as an indicator of the size of an IO.
>> + */
>> +void aio_kernel_init_iter(struct kiocb *iocb, struct file *filp,
>> + unsigned short op, struct iov_iter *iter, loff_t off)
>> +{
>> + iocb->ki_filp = filp;
>> + iocb->ki_iter = iter;
>> + iocb->ki_opcode = op;
>> + iocb->ki_pos = off;
>> + iocb->ki_nbytes = iov_iter_count(iter);
>> + iocb->ki_left = iocb->ki_nbytes;
>> + iocb->ki_ctx = (void *)-1;
>> +}
>> +EXPORT_SYMBOL_GPL(aio_kernel_init_iter);
>> +
>> void aio_kernel_init_callback(struct kiocb *iocb,
>> void (*complete)(u64 user_data, long res),
>> u64 user_data)
>> diff --git a/include/linux/aio.h b/include/linux/aio.h
>> index 014a75d..64d059d 100644
>> --- a/include/linux/aio.h
>> +++ b/include/linux/aio.h
>> @@ -66,6 +66,7 @@ struct kiocb {
>> * this is the underlying eventfd context to deliver events to.
>> */
>> struct eventfd_ctx *ki_eventfd;
>> + struct iov_iter *ki_iter;
>> };
>>
>> static inline bool is_sync_kiocb(struct kiocb *kiocb)
>> @@ -102,6 +103,8 @@ struct kiocb *aio_kernel_alloc(gfp_t gfp);
>> void aio_kernel_free(struct kiocb *iocb);
>> void aio_kernel_init_rw(struct kiocb *iocb, struct file *filp,
>> unsigned short op, void *ptr, size_t nr, loff_t off);
>> +void aio_kernel_init_iter(struct kiocb *iocb, struct file *filp,
>> + unsigned short op, struct iov_iter *iter, loff_t off);
>> void aio_kernel_init_callback(struct kiocb *iocb,
>> void (*complete)(u64 user_data, long res),
>> u64 user_data);
>> diff --git a/include/uapi/linux/aio_abi.h b/include/uapi/linux/aio_abi.h
>> index bb2554f..22ce4bd 100644
>> --- a/include/uapi/linux/aio_abi.h
>> +++ b/include/uapi/linux/aio_abi.h
>> @@ -44,6 +44,8 @@ enum {
>> IOCB_CMD_NOOP = 6,
>> IOCB_CMD_PREADV = 7,
>> IOCB_CMD_PWRITEV = 8,
>> + IOCB_CMD_READ_ITER = 9,
>> + IOCB_CMD_WRITE_ITER = 10,
>> };
>>
>> /*
>> --
>> 1.8.3.4
--
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