Date: Mon, 6 Feb 2023 14:19:38 -0800 From: John Baldwin <jhb@FreeBSD.org> To: "Bjoern A. Zeeb" <bz@FreeBSD.org>, src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: Re: git: 0fce2dc15730 - main - LinuxKPI,lindebugfs: add u8 base type and blob support Message-ID: <3e74025f-eedf-7ca2-a5a0-b3971dfdfe30@FreeBSD.org> In-Reply-To: <202211281724.2ASHOxU9080511@gitrepo.freebsd.org> References: <202211281724.2ASHOxU9080511@gitrepo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 11/28/22 9:24 AM, Bjoern A. Zeeb wrote: > The branch main has been updated by bz: > > URL: https://cgit.FreeBSD.org/src/commit/?id=0fce2dc1573019d0732f33fa7c26cc228655d3e8 > > commit 0fce2dc1573019d0732f33fa7c26cc228655d3e8 > Author: Bjoern A. Zeeb <bz@FreeBSD.org> > AuthorDate: 2022-10-22 18:12:16 +0000 > Commit: Bjoern A. Zeeb <bz@FreeBSD.org> > CommitDate: 2022-11-28 17:21:50 +0000 > > LinuxKPI,lindebugfs: add u8 base type and blob support > > Add debugfs_create_u8() based on other already present implementations. > Add a read-only implementation for debugfs_create_blob(). > > Both are needed for iwlwifi debugfs support. This passes kernel pointers to copyout because debugfs is kind of broken due to quirks in how linux file operations are implemented. > Sponsored by: The FreeBSD Foundation > MFC after: 3 days > OKed by: jfree (earlier version) > Differential Revision: https://reviews.freebsd.org/D37090 > --- > sys/compat/lindebugfs/lindebugfs.c | 66 ++++++++++++++++++++++ > sys/compat/linuxkpi/common/include/linux/debugfs.h | 11 +++- > 2 files changed, 76 insertions(+), 1 deletion(-) > > diff --git a/sys/compat/lindebugfs/lindebugfs.c b/sys/compat/lindebugfs/lindebugfs.c > index b88caf9c3140..b72ceb5e0be9 100644 > --- a/sys/compat/lindebugfs/lindebugfs.c > +++ b/sys/compat/lindebugfs/lindebugfs.c > @@ -394,6 +423,43 @@ debugfs_create_ulong(const char *name, umode_t mode, struct dentry *parent, unsi > &fops_ulong_ro, &fops_ulong_wo); > } > > + > +static ssize_t > +fops_blob_read(struct file *filp, char __user *ubuf, size_t read_size, loff_t *ppos) This is not actually a __user pointer. Note that simple_attr_read/write use plain pointers. The only invocations of this function are in debugfs_fill: rc = -ENODEV; if (uio->uio_rw == UIO_READ && d->dm_fops->read) { rc = -ENOMEM; buf = (char *) malloc(sb->s_size, M_DFSINT, M_ZERO | M_NOWAIT); if (buf != NULL) { rc = d->dm_fops->read(&lf, buf, sb->s_size, &off); if (rc > 0) sbuf_bcpy(sb, buf, strlen(buf)); free(buf, M_DFSINT); } } else if (uio->uio_rw == UIO_WRITE && d->dm_fops->write) { sbuf_finish(sb); rc = d->dm_fops->write(&lf, sbuf_data(sb), sbuf_len(sb), &off); } > +{ > + struct debugfs_blob_wrapper *blob; > + > + blob = filp->private_data; > + if (blob == NULL) > + return (-EINVAL); > + if (blob->size == 0 || blob->data == NULL) > + return (-EINVAL); > + > + return (simple_read_from_buffer(ubuf, read_size, ppos, blob->data, blob->size)); > +} This means you can't use simple_read_from_buffer() here as it is a wrapper around copyout. At least some architectures fail copyout calls where the user pointer is a kernel pointer (e.g. arm64 and RISC-V). I haven't looked if amd64 has the misfeature of letting it work. I'll have to fix this for CheriBSD downstream, but I'll try to post a review using an inlined version of simple_read_from_buffer that just uses memcpy directly. -- John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?3e74025f-eedf-7ca2-a5a0-b3971dfdfe30>