On Wed, 2020-09-30 at 21:17 -0700, Kees Cook wrote: > On Wed, Sep 30, 2020 at 01:57:40PM +0200, Greg Kroah-Hartman wrote: > > Kees, and Rafael, I don't know if you saw this proposal from Joe for > > sysfs files, questions below: > > I'm a fan. I think the use of sprintf() in sysfs might have been one of > my earliest complaints about unsafe code patterns in the kernel. ;) [] > > > + if (WARN(!buf || offset_in_page(buf), > > > + "invalid sysfs_emit: buf:%p\n", buf))
The dump_stack() is also going to emit pointers so I don't see how it does anything but help show where the buffer was. It is hashed... > I don't want the %p here, but otherwise, sure. I'd also make it a _ONCE > variant: > > if (WARN_ONCE(!buf || offset_in_page(buf), > "invalid sysfs_emit: offset_in_page(buf):%zd\n", > buf ? offset_in_page(buf) : 0)) I don't think that helps as multiple defects can easily exist. Log spam in this case isn't horrible either.
