On Thu, Jul 16, 2020 at 01:35:17PM -0700, Scott Branden wrote: > On 2020-07-10 3:44 p.m., Kees Cook wrote: > > On Fri, Jul 10, 2020 at 03:10:25PM -0700, Scott Branden wrote: > > > > > > On 2020-07-10 3:04 p.m., Matthew Wilcox wrote: > > > > On Fri, Jul 10, 2020 at 02:00:32PM -0700, Scott Branden wrote: > > > > > > @@ -950,8 +951,8 @@ int kernel_read_file(struct file *file, void **buf, loff_t *size, > > > > > > goto out; > > > > > > } > > > > > > - if (id != READING_FIRMWARE_PREALLOC_BUFFER) > > > > > > - *buf = vmalloc(i_size); > > > > > > + if (!*buf) > > > > > The assumption that *buf is always NULL when id != > > > > > READING_FIRMWARE_PREALLOC_BUFFER doesn't appear to be correct. > > > > > I get unhandled page faults due to this change on boot. > > > > Did it give you a stack backtrace? > > > Yes, but there's no requirement that *buf need to be NULL when calling this > > > function. > > > To fix my particular crash I added the following locally: > > > > > > --- a/kernel/module.c > > > +++ b/kernel/module.c > > > @@ -3989,7 +3989,7 @@ SYSCALL_DEFINE3(finit_module, int, fd, const char > > > __user *, uargs, int, flags) > > > { > > > struct load_info info = { }; > > > loff_t size; > > > - void *hdr; > > > + void *hdr = NULL; > > > int err; > > > > > > err = may_init_module(); > > Thanks for the diagnosis and fix! I haven't had time to cycle back > > around to this series yet. Hopefully soon. :) > > > In order to assist in your patchset I have combined it with my patch series > here: > https://github.com/sbranden/linux/tree/kernel_read_file_for_kees > > Please let me know if this matches your expectations for my patches or if > there is something else I need to change. Thanks! I was working on the next revision of this last night, and I'm trying to get through today's email to finish it. I'll take a look! -- Kees Cook