* [PATCH] dax: explain how read(2)/write(2) addresses are validated
[not found] <CAGXu5jJqoegDg07vR0iRRV1U_qOpZx5eLbywEwD_sm-D7sQmdA@mail.gmail.com>
@ 2017-08-16 17:36 ` Ross Zwisler
0 siblings, 0 replies; 6+ messages in thread
From: Ross Zwisler @ 2017-08-16 17:36 UTC (permalink / raw)
To: Andrew Morton, linux-kernel
Cc: Mark Rutland, Jan Kara, Kees Cook, Matthew Wilcox, Will Deacon,
linux-nvdimm, Dave Chinner, Christoph Hellwig, Alexander Viro,
Catalin Marinas, linux-fsdevel, Robin Murphy
Add a comment explaining how the user addresses provided to read(2) and
write(2) are validated in the DAX I/O path. We call dax_copy_from_iter()
or copy_to_iter() on these without calling access_ok() first in the DAX
code, and there was a concern that the user might be able to read/write to
arbitrary kernel addresses with this path.
Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
---
Adding a comment instead of adding redundant access_ok() calls in the DAX
code. If this is the wrong path to take, please let me know.
fs/dax.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/fs/dax.c b/fs/dax.c
index 8c67517..2d50f32 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -1060,6 +1060,11 @@ dax_iomap_actor(struct inode *inode, loff_t pos, loff_t length, void *data,
if (map_len > end - pos)
map_len = end - pos;
+ /*
+ * The userspace address for the memory copy has already been
+ * validated via access_ok() in either vfs_read() or
+ * vfs_write(), depending on which operation we are doing.
+ */
if (iov_iter_rw(iter) == WRITE)
map_len = dax_copy_from_iter(dax_dev, pgoff, kaddr,
map_len, iter);
--
2.9.5
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH] dax: explain how read(2)/write(2) addresses are validated
@ 2017-08-16 17:36 ` Ross Zwisler
0 siblings, 0 replies; 6+ messages in thread
From: Ross Zwisler @ 2017-08-16 17:36 UTC (permalink / raw)
To: Andrew Morton, linux-kernel
Cc: Ross Zwisler, Alexander Viro, Christoph Hellwig, Dan Williams,
Dave Chinner, Jan Kara, Matthew Wilcox, linux-fsdevel,
linux-nvdimm, Kees Cook, Will Deacon, Catalin Marinas,
Robin Murphy, Mark Rutland
Add a comment explaining how the user addresses provided to read(2) and
write(2) are validated in the DAX I/O path. We call dax_copy_from_iter()
or copy_to_iter() on these without calling access_ok() first in the DAX
code, and there was a concern that the user might be able to read/write to
arbitrary kernel addresses with this path.
Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
---
Adding a comment instead of adding redundant access_ok() calls in the DAX
code. If this is the wrong path to take, please let me know.
fs/dax.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/fs/dax.c b/fs/dax.c
index 8c67517..2d50f32 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -1060,6 +1060,11 @@ dax_iomap_actor(struct inode *inode, loff_t pos, loff_t length, void *data,
if (map_len > end - pos)
map_len = end - pos;
+ /*
+ * The userspace address for the memory copy has already been
+ * validated via access_ok() in either vfs_read() or
+ * vfs_write(), depending on which operation we are doing.
+ */
if (iov_iter_rw(iter) == WRITE)
map_len = dax_copy_from_iter(dax_dev, pgoff, kaddr,
map_len, iter);
--
2.9.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] dax: explain how read(2)/write(2) addresses are validated
2017-08-16 17:36 ` Ross Zwisler
@ 2017-08-17 8:53 ` Jan Kara
-1 siblings, 0 replies; 6+ messages in thread
From: Jan Kara @ 2017-08-17 8:53 UTC (permalink / raw)
To: Ross Zwisler
Cc: Mark Rutland, Jan Kara, Kees Cook, Matthew Wilcox, Will Deacon,
linux-nvdimm, Dave Chinner, linux-kernel, Alexander Viro,
Catalin Marinas, linux-fsdevel, Andrew Morton, Robin Murphy,
Christoph Hellwig
On Wed 16-08-17 11:36:15, Ross Zwisler wrote:
> Add a comment explaining how the user addresses provided to read(2) and
> write(2) are validated in the DAX I/O path. We call dax_copy_from_iter()
> or copy_to_iter() on these without calling access_ok() first in the DAX
> code, and there was a concern that the user might be able to read/write to
> arbitrary kernel addresses with this path.
>
> Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Looks OK to me so feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Just I'd note that standard buffered read / write path is no different so I
don't see a big point in adding this comment when it is not in any other
path either...
Honza
> ---
>
> Adding a comment instead of adding redundant access_ok() calls in the DAX
> code. If this is the wrong path to take, please let me know.
>
> fs/dax.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/fs/dax.c b/fs/dax.c
> index 8c67517..2d50f32 100644
> --- a/fs/dax.c
> +++ b/fs/dax.c
> @@ -1060,6 +1060,11 @@ dax_iomap_actor(struct inode *inode, loff_t pos, loff_t length, void *data,
> if (map_len > end - pos)
> map_len = end - pos;
>
> + /*
> + * The userspace address for the memory copy has already been
> + * validated via access_ok() in either vfs_read() or
> + * vfs_write(), depending on which operation we are doing.
> + */
> if (iov_iter_rw(iter) == WRITE)
> map_len = dax_copy_from_iter(dax_dev, pgoff, kaddr,
> map_len, iter);
> --
> 2.9.5
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] dax: explain how read(2)/write(2) addresses are validated
@ 2017-08-17 8:53 ` Jan Kara
0 siblings, 0 replies; 6+ messages in thread
From: Jan Kara @ 2017-08-17 8:53 UTC (permalink / raw)
To: Ross Zwisler
Cc: Andrew Morton, linux-kernel, Alexander Viro, Christoph Hellwig,
Dan Williams, Dave Chinner, Jan Kara, Matthew Wilcox,
linux-fsdevel, linux-nvdimm, Kees Cook, Will Deacon,
Catalin Marinas, Robin Murphy, Mark Rutland
On Wed 16-08-17 11:36:15, Ross Zwisler wrote:
> Add a comment explaining how the user addresses provided to read(2) and
> write(2) are validated in the DAX I/O path. We call dax_copy_from_iter()
> or copy_to_iter() on these without calling access_ok() first in the DAX
> code, and there was a concern that the user might be able to read/write to
> arbitrary kernel addresses with this path.
>
> Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Looks OK to me so feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Just I'd note that standard buffered read / write path is no different so I
don't see a big point in adding this comment when it is not in any other
path either...
Honza
> ---
>
> Adding a comment instead of adding redundant access_ok() calls in the DAX
> code. If this is the wrong path to take, please let me know.
>
> fs/dax.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/fs/dax.c b/fs/dax.c
> index 8c67517..2d50f32 100644
> --- a/fs/dax.c
> +++ b/fs/dax.c
> @@ -1060,6 +1060,11 @@ dax_iomap_actor(struct inode *inode, loff_t pos, loff_t length, void *data,
> if (map_len > end - pos)
> map_len = end - pos;
>
> + /*
> + * The userspace address for the memory copy has already been
> + * validated via access_ok() in either vfs_read() or
> + * vfs_write(), depending on which operation we are doing.
> + */
> if (iov_iter_rw(iter) == WRITE)
> map_len = dax_copy_from_iter(dax_dev, pgoff, kaddr,
> map_len, iter);
> --
> 2.9.5
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] dax: explain how read(2)/write(2) addresses are validated
2017-08-17 8:53 ` Jan Kara
@ 2017-08-17 15:20 ` Ross Zwisler
-1 siblings, 0 replies; 6+ messages in thread
From: Ross Zwisler @ 2017-08-17 15:20 UTC (permalink / raw)
To: Jan Kara
Cc: Mark Rutland, Andrew Morton, Matthew Wilcox, Will Deacon,
linux-nvdimm, Dave Chinner, linux-kernel, Christoph Hellwig,
Alexander Viro, Catalin Marinas, linux-fsdevel, Robin Murphy,
Kees Cook
On Thu, Aug 17, 2017 at 10:53:32AM +0200, Jan Kara wrote:
> On Wed 16-08-17 11:36:15, Ross Zwisler wrote:
> > Add a comment explaining how the user addresses provided to read(2) and
> > write(2) are validated in the DAX I/O path. We call dax_copy_from_iter()
> > or copy_to_iter() on these without calling access_ok() first in the DAX
> > code, and there was a concern that the user might be able to read/write to
> > arbitrary kernel addresses with this path.
> >
> > Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
>
> Looks OK to me so feel free to add:
>
> Reviewed-by: Jan Kara <jack@suse.cz>
>
> Just I'd note that standard buffered read / write path is no different so I
> don't see a big point in adding this comment when it is not in any other
> path either...
Fair enough. Yea, if it's not in any of the other paths either and it's just
common knowledge that these addresses are validated at the VFS layer, we can
leave it out.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] dax: explain how read(2)/write(2) addresses are validated
@ 2017-08-17 15:20 ` Ross Zwisler
0 siblings, 0 replies; 6+ messages in thread
From: Ross Zwisler @ 2017-08-17 15:20 UTC (permalink / raw)
To: Jan Kara
Cc: Ross Zwisler, Andrew Morton, linux-kernel, Alexander Viro,
Christoph Hellwig, Dan Williams, Dave Chinner, Matthew Wilcox,
linux-fsdevel, linux-nvdimm, Kees Cook, Will Deacon,
Catalin Marinas, Robin Murphy, Mark Rutland
On Thu, Aug 17, 2017 at 10:53:32AM +0200, Jan Kara wrote:
> On Wed 16-08-17 11:36:15, Ross Zwisler wrote:
> > Add a comment explaining how the user addresses provided to read(2) and
> > write(2) are validated in the DAX I/O path. We call dax_copy_from_iter()
> > or copy_to_iter() on these without calling access_ok() first in the DAX
> > code, and there was a concern that the user might be able to read/write to
> > arbitrary kernel addresses with this path.
> >
> > Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
>
> Looks OK to me so feel free to add:
>
> Reviewed-by: Jan Kara <jack@suse.cz>
>
> Just I'd note that standard buffered read / write path is no different so I
> don't see a big point in adding this comment when it is not in any other
> path either...
Fair enough. Yea, if it's not in any of the other paths either and it's just
common knowledge that these addresses are validated at the VFS layer, we can
leave it out.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-08-17 15:20 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CAGXu5jJqoegDg07vR0iRRV1U_qOpZx5eLbywEwD_sm-D7sQmdA@mail.gmail.com>
2017-08-16 17:36 ` [PATCH] dax: explain how read(2)/write(2) addresses are validated Ross Zwisler
2017-08-16 17:36 ` Ross Zwisler
2017-08-17 8:53 ` Jan Kara
2017-08-17 8:53 ` Jan Kara
2017-08-17 15:20 ` Ross Zwisler
2017-08-17 15:20 ` Ross Zwisler
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.