lindebugfs: Pass user buffer pointers to the read/write file operations

The Linux file_operations API expects the read and write operations
to take a single user buffer pointer (along with the length and the
file offset as an in/out parameter).

However, the debugfs_fill function was violating this part of the
contract as it was passing down kernel pointers instead.  An earlier
commit (5668c22a13c6befa9b8486387d38457c40ce7af4) hacked around this
by modifying simple_read_from_buffer() to treat its user pointer
argument as a kernel pointer instead.  However, other commits keep
tripping over this same API mismatch
(e.g. 78e25e65bf381303c8bdac9a713ab7b26a854b8c passes a kernel pointer
to copy_from_user in fops_str_write).

Instead, change debugfs_fill to use the "raw" pseudofs mode where the
uio is passed down to directly to the fill callback rather than an
sbuf.  debufs_fill now iterates over the iovec in the uio similar to
the implementation of uiomove invoking the read or write operation on
each user pointer.

This also fixes a tiny bug where the initial file offset from
uio_offset was ignored.  Instead, the operations were always invoked
with a file offset of 0.

As part of this, revert the the changes to simple_read_from_buffer()
from commit 5668c22a13c6befa9b8486387d38457c40ce7af4.

Also as part of this, the simple_attr_read/write methods and seq_read
now also need to accept and handle user pointers (also matching the
API in Linux).

For simple_attr_write*(), copy the user buffer into a kernel buffer
before parsing.  Also, do not permit writes at an offset as it's
unclear what the semantics for those would even be (perhaps you would
write out the formatted value into a buffer first and then allow the
copy_from_user to overwrite/extend that buffer and then re-parse the
integer value?).  The old handling of *ppos for writes was definitely
wrong before and only worked for an offset of 0 anyway.

Reviewed by:	bz
Sponsored by:	AFRL, DARPA
Differential Revision:	https://reviews.freebsd.org/D55833

2 files changed, 14 insertions, 16 deletions

diff --git a/sys/compat/linuxkpi/common/include/linux/fs.h b/sys/compat/linuxkpi/common/include/linux/fs.h
index f1568ad6282d..40e1b396fe86 100644
--- a/sys/compat/linuxkpi/common/include/linux/fs.h
+++ b/sys/compat/linuxkpi/common/include/linux/fs.h
@@ -364,8 +364,9 @@ static inline ssize_t
 simple_read_from_buffer(void __user *dest, size_t read_size, loff_t *ppos,
     void *orig, size_t buf_size)
 {
-	void *p, *read_pos = ((char *) orig) + *ppos;
+	void *read_pos = ((char *) orig) + *ppos;
 	size_t buf_remain = buf_size - *ppos;
+	ssize_t num_read;
 
 	if (buf_remain < 0 || buf_remain > buf_size)
 		return -EINVAL;
@@ -373,18 +374,13 @@ simple_read_from_buffer(void __user *dest, size_t read_size, loff_t *ppos,
 	if (read_size > buf_remain)
 		read_size = buf_remain;
 
-	/*
-	 * XXX At time of commit only debugfs consumers could be
-	 * identified.  If others will use this function we may
-	 * have to revise this: normally we would call copy_to_user()
-	 * here but lindebugfs will return the result and the
-	 * copyout is done elsewhere for us.
-	 */
-	p = memcpy(dest, read_pos, read_size);
-	if (p != NULL)
-		*ppos += read_size;
+	/* copy_to_user returns number of bytes NOT read */
+	num_read = read_size - copy_to_user(dest, read_pos, read_size);
+	if (num_read == 0)
+		return -EFAULT;
+	*ppos += num_read;
 
-	return (read_size);
+	return (num_read);
 }
 
 MALLOC_DECLARE(M_LSATTR);
@@ -415,11 +411,13 @@ int simple_attr_open(struct inode *inode, struct file *filp,
 
 int simple_attr_release(struct inode *inode, struct file *filp);
 
-ssize_t simple_attr_read(struct file *filp, char *buf, size_t read_size, loff_t *ppos);
+ssize_t simple_attr_read(struct file *filp, char __user *buf, size_t read_size,
+    loff_t *ppos);
 
-ssize_t simple_attr_write(struct file *filp, const char *buf, size_t write_size, loff_t *ppos);
+ssize_t simple_attr_write(struct file *filp, const char __user *buf,
+    size_t write_size, loff_t *ppos);
 
-ssize_t simple_attr_write_signed(struct file *filp, const char *buf,
+ssize_t simple_attr_write_signed(struct file *filp, const char __user *buf,
 	    size_t write_size, loff_t *ppos);
 
 #endif /* _LINUXKPI_LINUX_FS_H_ */
diff --git a/sys/compat/linuxkpi/common/include/linux/seq_file.h b/sys/compat/linuxkpi/common/include/linux/seq_file.h
index 3c7862890c67..786c09bd6a20 100644
--- a/sys/compat/linuxkpi/common/include/linux/seq_file.h
+++ b/sys/compat/linuxkpi/common/include/linux/seq_file.h
@@ -85,7 +85,7 @@ struct seq_operations {
 	int (*show) (struct seq_file *m, void *v);
 };
 
-ssize_t seq_read(struct linux_file *, char *, size_t, off_t *);
+ssize_t seq_read(struct linux_file *, char __user *, size_t, off_t *);
 int seq_write(struct seq_file *seq, const void *data, size_t len);
 void seq_putc(struct seq_file *m, char c);
 void seq_puts(struct seq_file *m, const char *str);