On Tue, Apr 30, 2019 at 2:47 PM Sami Tolvanen <samitolvanen@xxxxxxxxxx> wrote: > > Casting mapping->a_ops->readpage to filler_t causes an indirect call > type mismatch with Control-Flow Integrity checking. This change fixes > the mismatch in read_cache_page_gfp and read_mapping_page by adding a > stub callback function with the correct type. > > As the kernel only has a couple of instances of read_cache_page(s) > being called with a callback function that doesn't accept struct file* > as the first parameter, Android kernels have previously fixed this by > changing filler_t to int (*filler_t)(struct file *, struct page *): > > https://android-review.googlesource.com/c/kernel/common/+/671260 > > While this approach did fix most of the issues, the few remaining > cases where unrelated private data are passed to the callback become > rather awkward. Keeping filler_t unchanged and using a stub function > for readpage instead solves this problem. > > Cc: Kees Cook <keescook@xxxxxxxxxxxx> > Signed-off-by: Sami Tolvanen <samitolvanen@xxxxxxxxxx> Reviewed-by: Kees Cook <keescook@xxxxxxxxxxxx> -Kees > --- > include/linux/pagemap.h | 22 +++++++++++++++++++--- > mm/filemap.c | 7 +++++-- > 2 files changed, 24 insertions(+), 5 deletions(-) > > diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h > index bcf909d0de5f8..e5652a5ba1584 100644 > --- a/include/linux/pagemap.h > +++ b/include/linux/pagemap.h > @@ -383,11 +383,27 @@ extern struct page * read_cache_page_gfp(struct address_space *mapping, > extern int read_cache_pages(struct address_space *mapping, > struct list_head *pages, filler_t *filler, void *data); > > +struct file_filler_data { > + int (*filler)(struct file *, struct page *); > + struct file *filp; > +}; > + > +static inline int __file_filler(void *data, struct page *page) > +{ > + struct file_filler_data *ffd = (struct file_filler_data *)data; > + > + return ffd->filler(ffd->filp, page); > +} > + > static inline struct page *read_mapping_page(struct address_space *mapping, > - pgoff_t index, void *data) > + pgoff_t index, struct file *filp) > { > - filler_t *filler = (filler_t *)mapping->a_ops->readpage; > - return read_cache_page(mapping, index, filler, data); > + struct file_filler_data data = { > + .filler = mapping->a_ops->readpage, > + .filp = filp > + }; > + > + return read_cache_page(mapping, index, __file_filler, &data); > } > > /* > diff --git a/mm/filemap.c b/mm/filemap.c > index d78f577baef2a..6cc41c25ca3bf 100644 > --- a/mm/filemap.c > +++ b/mm/filemap.c > @@ -2977,9 +2977,12 @@ struct page *read_cache_page_gfp(struct address_space *mapping, > pgoff_t index, > gfp_t gfp) > { > - filler_t *filler = (filler_t *)mapping->a_ops->readpage; > + struct file_filler_data data = { > + .filler = mapping->a_ops->readpage, > + .filp = NULL > + }; > > - return do_read_cache_page(mapping, index, filler, NULL, gfp); > + return do_read_cache_page(mapping, index, __file_filler, &data, gfp); > } > EXPORT_SYMBOL(read_cache_page_gfp); > > -- > 2.21.0.593.g511ec345e18-goog > -- Kees Cook
