mm: fix filler_t callback type mismatch with readpage
Subsystems (2)
PATCH 1/1 mm: fix filler_t callback type mismatch with readpage
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.
--- include/linux/pagemap.h | 22 +++++++++++++++++++--- mm/filemap.c | 7 +++++-- 2 files changed, 24 insertions(+), 5 deletions(-)
include/linux/pagemap.h +19 -3
@@ -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); }/*
mm/filemap.c +5 -2
@@ -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);
Signature · 1 line
2.21.0.593.g511ec345e18-goog
Quoted 13 lines · Sami Tolvanen
Casting
mapping->a_ops->readpagetofiller_tcauses an indirect call type mismatch with Control-Flow Integrity checking. This change fixes the mismatch inread_cache_page_gfpandread_mapping_pageby 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 changingfiller_tto int (*filler_t)(struct file *, struct page *):https://android-review.googlesource.com/c/kernel/common/+/671260
Quoted 7 lines
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_tunchanged and using a stub function for readpage instead solves this problem.Cc Kees Cook <[email protected]>Signed-off-by Sami Tolvanen <[email protected]>
-Kees
Quoted 62 lines
--- include/linux/pagemap.h | 22 +++++++++++++++++++--- mm/filemap.c | 7 +++++-- 2 files changed, 24 insertions(+), 5 deletions(-)include/linux/pagemap.h +19
-3@@ -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); }/* mm/filemap.c +5
-3@@ -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
Signature · 1 line
Kees Cook
This still leaves bugs around in jffs2 and nfs. And it is a little ugly. This is what I'd like to do instead, so far untested. I'll post a series once it passes basic testing:
http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/filler-fixes