From 2cf15144daf7ec44cdcd9bf3ef007939b79c361e Mon Sep 17 00:00:00 2001 From: John Baldwin Date: Tue, 17 Mar 2026 15:45:34 -0400 Subject: 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 --- sys/compat/linuxkpi/common/include/linux/fs.h | 30 +++++++++++++-------------- 1 file changed, 14 insertions(+), 16 deletions(-) (limited to 'sys/compat/linuxkpi/common/include/linux/fs.h') 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; - - return (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 (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_ */ -- cgit v1.3