Passing kernel-allocated buffers into lindebugfs fops causes unintended behavior and EFAULT. Instead, pass userspace buffers into lindebugfs ->read() and ->write() to properly reproduce Linux behavior. This patch also updates simple_attr_file and seq_file implementations to support this change.
Details
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Skipped - Unit
Tests Skipped
Event Timeline
sys/compat/lindebugfs/lindebugfs.c | ||
---|---|---|
151 | buf = sbuf_data(sb); /* On write I think we expect kernel memory and not user space? */ /* Fixes my panic (1) as I realized after sending email */ |
Use copyout() instead of memcpy() for lindebugfs's kernel sb to userspace buf transfer.
sys/compat/lindebugfs/lindebugfs.c | ||
---|---|---|
146 | We lost a check here to only call sbuf_bcopyin if rc > 0 and not on -ERROR cases. |
Add check to make sure ->read() > 0 before copying to sbuf.
Thanks to @bz for pointing this out.
Hey @bz, what is the status for passing userspace buffers into write
fops for iwlwifi? DRM drivers expect a userspace buffer. I know you
mentioned earlier that iwlwifi expects a kernel buffer?
sys/compat/lindebugfs/lindebugfs.c | ||
---|---|---|
151 | This is not a great solution. Pseudofs, the lindebugfs backend uses sbuf_uionew() in pseudofs_vnops.c:1126 which transfers the original uio vector data into a kernel space sbuf. Since we need that data in a userspace vector, I use memcpy() to send the data back into the uio_iov[0].iov_base. This is bad because
This could be avoided by directly modifying pseudofs_vnops.c, but I do not think it is possible to detect if the write operation is being sent to lindebugfs or another pseudo file system. This would also add a random exception into generic pseudofs code, which is not great. Any suggestions would be appreciated. | |
151 | That is strange. The Linux simple_attr spec calls for a userspace buffer in its write. Passing in the kernel buffer fixes your panic, if I understand correctly? If that is the case, we have conflicting code. In drm-kmod's i915_debugfs.c:745, the i915_error_state_write() expects a userspace buffer according to the function notation. There are many other examples in drm drivers that expect this to be a user buffer as well. I wonder how Linux handles this? |