* [PATCH 1/4] mm: fix an overly long line in read_cache_page
2019-05-20 5:57 fix filler_t callback type mismatches v2 Christoph Hellwig
@ 2019-05-20 5:57 ` Christoph Hellwig
2019-05-20 5:57 ` [PATCH 2/4] mm: don't cast ->readpage to filler_t for do_read_cache_page Christoph Hellwig
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Christoph Hellwig @ 2019-05-20 5:57 UTC (permalink / raw)
To: Andrew Morton
Cc: Sami Tolvanen, Kees Cook, Nick Desaulniers, linux-mtd, linux-nfs,
linux-mm, linux-kernel
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Kees Cook <keescook@chromium.org>
---
mm/filemap.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/mm/filemap.c b/mm/filemap.c
index c5af80c43d36..6a8048477bc6 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -2862,7 +2862,8 @@ struct page *read_cache_page(struct address_space *mapping,
int (*filler)(void *, struct page *),
void *data)
{
- return do_read_cache_page(mapping, index, filler, data, mapping_gfp_mask(mapping));
+ return do_read_cache_page(mapping, index, filler, data,
+ mapping_gfp_mask(mapping));
}
EXPORT_SYMBOL(read_cache_page);
--
2.20.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/4] mm: don't cast ->readpage to filler_t for do_read_cache_page
2019-05-20 5:57 fix filler_t callback type mismatches v2 Christoph Hellwig
2019-05-20 5:57 ` [PATCH 1/4] mm: fix an overly long line in read_cache_page Christoph Hellwig
@ 2019-05-20 5:57 ` Christoph Hellwig
2019-05-20 5:57 ` [PATCH 3/4] jffs2: pass the correct prototype to read_cache_page Christoph Hellwig
2019-05-20 5:57 ` [PATCH 4/4] 9p: " Christoph Hellwig
3 siblings, 0 replies; 7+ messages in thread
From: Christoph Hellwig @ 2019-05-20 5:57 UTC (permalink / raw)
To: Andrew Morton
Cc: Sami Tolvanen, Kees Cook, Nick Desaulniers, linux-mtd, linux-nfs,
linux-mm, linux-kernel
We can just pass a NULL filler and do the right thing inside of
do_read_cache_page based on the NULL parameter.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Kees Cook <keescook@chromium.org>
---
include/linux/pagemap.h | 3 +--
mm/filemap.c | 10 ++++++----
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
index 9ec3544baee2..6dd7ec95c778 100644
--- a/include/linux/pagemap.h
+++ b/include/linux/pagemap.h
@@ -396,8 +396,7 @@ extern int read_cache_pages(struct address_space *mapping,
static inline struct page *read_mapping_page(struct address_space *mapping,
pgoff_t index, void *data)
{
- filler_t *filler = (filler_t *)mapping->a_ops->readpage;
- return read_cache_page(mapping, index, filler, data);
+ return read_cache_page(mapping, index, NULL, data);
}
/*
diff --git a/mm/filemap.c b/mm/filemap.c
index 6a8048477bc6..3bec6e18b763 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -2772,7 +2772,11 @@ static struct page *do_read_cache_page(struct address_space *mapping,
}
filler:
- err = filler(data, page);
+ if (filler)
+ err = filler(data, page);
+ else
+ err = mapping->a_ops->readpage(data, page);
+
if (err < 0) {
put_page(page);
return ERR_PTR(err);
@@ -2884,9 +2888,7 @@ 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;
-
- return do_read_cache_page(mapping, index, filler, NULL, gfp);
+ return do_read_cache_page(mapping, index, NULL, NULL, gfp);
}
EXPORT_SYMBOL(read_cache_page_gfp);
--
2.20.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/4] jffs2: pass the correct prototype to read_cache_page
2019-05-20 5:57 fix filler_t callback type mismatches v2 Christoph Hellwig
2019-05-20 5:57 ` [PATCH 1/4] mm: fix an overly long line in read_cache_page Christoph Hellwig
2019-05-20 5:57 ` [PATCH 2/4] mm: don't cast ->readpage to filler_t for do_read_cache_page Christoph Hellwig
@ 2019-05-20 5:57 ` Christoph Hellwig
2019-06-18 20:27 ` Al Viro
2019-05-20 5:57 ` [PATCH 4/4] 9p: " Christoph Hellwig
3 siblings, 1 reply; 7+ messages in thread
From: Christoph Hellwig @ 2019-05-20 5:57 UTC (permalink / raw)
To: Andrew Morton
Cc: Sami Tolvanen, Kees Cook, Nick Desaulniers, linux-mtd, linux-nfs,
linux-mm, linux-kernel
Fix the callback jffs2 passes to read_cache_page to actually have the
proper type expected. Casting around function pointers can easily
hide typing bugs, and defeats control flow protection.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Kees Cook <keescook@chromium.org>
---
fs/jffs2/file.c | 4 ++--
fs/jffs2/fs.c | 2 +-
fs/jffs2/os-linux.h | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/fs/jffs2/file.c b/fs/jffs2/file.c
index 7d8654a1472e..f8fb89b10227 100644
--- a/fs/jffs2/file.c
+++ b/fs/jffs2/file.c
@@ -109,9 +109,9 @@ static int jffs2_do_readpage_nolock (struct inode *inode, struct page *pg)
return ret;
}
-int jffs2_do_readpage_unlock(struct inode *inode, struct page *pg)
+int jffs2_do_readpage_unlock(void *data, struct page *pg)
{
- int ret = jffs2_do_readpage_nolock(inode, pg);
+ int ret = jffs2_do_readpage_nolock(data, pg);
unlock_page(pg);
return ret;
}
diff --git a/fs/jffs2/fs.c b/fs/jffs2/fs.c
index 112d85849db1..8a20ddd25f2d 100644
--- a/fs/jffs2/fs.c
+++ b/fs/jffs2/fs.c
@@ -687,7 +687,7 @@ unsigned char *jffs2_gc_fetch_page(struct jffs2_sb_info *c,
struct page *pg;
pg = read_cache_page(inode->i_mapping, offset >> PAGE_SHIFT,
- (void *)jffs2_do_readpage_unlock, inode);
+ jffs2_do_readpage_unlock, inode);
if (IS_ERR(pg))
return (void *)pg;
diff --git a/fs/jffs2/os-linux.h b/fs/jffs2/os-linux.h
index a2dbbb3f4c74..bd3d5f0ddc34 100644
--- a/fs/jffs2/os-linux.h
+++ b/fs/jffs2/os-linux.h
@@ -155,7 +155,7 @@ extern const struct file_operations jffs2_file_operations;
extern const struct inode_operations jffs2_file_inode_operations;
extern const struct address_space_operations jffs2_file_address_operations;
int jffs2_fsync(struct file *, loff_t, loff_t, int);
-int jffs2_do_readpage_unlock (struct inode *inode, struct page *pg);
+int jffs2_do_readpage_unlock(void *data, struct page *pg);
/* ioctl.c */
long jffs2_ioctl(struct file *, unsigned int, unsigned long);
--
2.20.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 3/4] jffs2: pass the correct prototype to read_cache_page
2019-05-20 5:57 ` [PATCH 3/4] jffs2: pass the correct prototype to read_cache_page Christoph Hellwig
@ 2019-06-18 20:27 ` Al Viro
0 siblings, 0 replies; 7+ messages in thread
From: Al Viro @ 2019-06-18 20:27 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Andrew Morton, Sami Tolvanen, Kees Cook, Nick Desaulniers,
linux-mtd, linux-nfs, linux-mm, linux-kernel
On Mon, May 20, 2019 at 07:57:30AM +0200, Christoph Hellwig wrote:
> Fix the callback jffs2 passes to read_cache_page to actually have the
> proper type expected. Casting around function pointers can easily
> hide typing bugs, and defeats control flow protection.
FWIW, this
unsigned char *jffs2_gc_fetch_page(struct jffs2_sb_info *c,
struct jffs2_inode_info *f,
unsigned long offset,
unsigned long *priv)
{
struct inode *inode = OFNI_EDONI_2SFFJ(f);
struct page *pg;
pg = read_cache_page(inode->i_mapping, offset >> PAGE_SHIFT,
(void *)jffs2_do_readpage_unlock, inode);
if (IS_ERR(pg))
return (void *)pg;
*priv = (unsigned long)pg;
return kmap(pg);
}
looks like crap. And so does this:
void jffs2_gc_release_page(struct jffs2_sb_info *c,
unsigned char *ptr,
unsigned long *priv)
{
struct page *pg = (void *)*priv;
kunmap(pg);
put_page(pg);
}
First of all, there's only one caller for each of those, and both
are direct calls. So passing struct page * around that way is ridiculous.
What's more, there is no reason not to do kmap() in caller (i.e. in
jffs2_garbage_collect_dnode()). That way jffs2_gc_fetch_page() would
simply be return read_cache_page(....), and in the caller we'd have
struct page *pg;
unsigned char *pg_ptr;
...
mutex_unlock(&f->sem);
pg = jffs2_gc_fetch_page(c, f, start);
if (IS_ERR(pg)) {
mutex_lock(&f->sem);
pr_warn("read_cache_page() returned error: %ld\n", PTR_ERR(pg));
return PTR_ERR(pg);
}
pg_ptr = kmap(pg);
mutex_lock(&f->sem);
...
kunmap(pg);
put_page(pg);
and that's it, preserving the current locking and with saner types...
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 4/4] 9p: pass the correct prototype to read_cache_page
2019-05-20 5:57 fix filler_t callback type mismatches v2 Christoph Hellwig
` (2 preceding siblings ...)
2019-05-20 5:57 ` [PATCH 3/4] jffs2: pass the correct prototype to read_cache_page Christoph Hellwig
@ 2019-05-20 5:57 ` Christoph Hellwig
3 siblings, 0 replies; 7+ messages in thread
From: Christoph Hellwig @ 2019-05-20 5:57 UTC (permalink / raw)
To: Andrew Morton
Cc: Sami Tolvanen, Kees Cook, Nick Desaulniers, linux-mtd, linux-nfs,
linux-mm, linux-kernel
Fix the callback 9p passes to read_cache_page to actually have the
proper type expected. Casting around function pointers can easily
hide typing bugs, and defeats control flow protection.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Kees Cook <keescook@chromium.org>
---
fs/9p/vfs_addr.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/fs/9p/vfs_addr.c b/fs/9p/vfs_addr.c
index 0bcbcc20f769..02e0fc51401e 100644
--- a/fs/9p/vfs_addr.c
+++ b/fs/9p/vfs_addr.c
@@ -50,8 +50,9 @@
* @page: structure to page
*
*/
-static int v9fs_fid_readpage(struct p9_fid *fid, struct page *page)
+static int v9fs_fid_readpage(void *data, struct page *page)
{
+ struct p9_fid *fid = data;
struct inode *inode = page->mapping->host;
struct bio_vec bvec = {.bv_page = page, .bv_len = PAGE_SIZE};
struct iov_iter to;
@@ -122,7 +123,8 @@ static int v9fs_vfs_readpages(struct file *filp, struct address_space *mapping,
if (ret == 0)
return ret;
- ret = read_cache_pages(mapping, pages, (void *)v9fs_vfs_readpage, filp);
+ ret = read_cache_pages(mapping, pages, v9fs_fid_readpage,
+ filp->private_data);
p9_debug(P9_DEBUG_VFS, " = %d\n", ret);
return ret;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 7+ messages in thread