| Message ID | 20220426082549.590899-1-wanjiabing@vivo.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm/filemap: Fix NULL pointer dereference in pagecache_get_page | expand |
On Tue, Apr 26, 2022 at 04:25:48PM +0800, Wan Jiabing wrote: > Fix following coccicheck error: > mm/folio-compat.c:128:17-21: ERROR: folio is NULL but dereferenced. > folio = __filemap_get_folio(mapping, index, fgp_flags, gfp); > - if ((fgp_flags & FGP_HEAD) || !folio || xa_is_value(folio)) > + if (!folio) > + return NULL; > + if ((fgp_flags & FGP_HEAD) || xa_is_value(folio)) > return &folio->page; That doesn't dereference the folio. Coccicheck is wrong.
On Tue, 26 Apr 2022 13:08:08 +0100 Matthew Wilcox <willy@infradead.org> wrote: > On Tue, Apr 26, 2022 at 04:25:48PM +0800, Wan Jiabing wrote: > > Fix following coccicheck error: > > mm/folio-compat.c:128:17-21: ERROR: folio is NULL but dereferenced. > > folio = __filemap_get_folio(mapping, index, fgp_flags, gfp); > > - if ((fgp_flags & FGP_HEAD) || !folio || xa_is_value(folio)) > > + if (!folio) > > + return NULL; > > + if ((fgp_flags & FGP_HEAD) || xa_is_value(folio)) > > return &folio->page; > > That doesn't dereference the folio. Coccicheck is wrong. Doing return &(0->page); is a rather obscure way of doing `return NULL;'. I agree the patch doesn't fix anything, but it results in saner-looking code?
On Tue, Apr 26, 2022 at 02:06:41PM -0700, Andrew Morton wrote: > On Tue, 26 Apr 2022 13:08:08 +0100 Matthew Wilcox <willy@infradead.org> wrote: > > > On Tue, Apr 26, 2022 at 04:25:48PM +0800, Wan Jiabing wrote: > > > Fix following coccicheck error: > > > mm/folio-compat.c:128:17-21: ERROR: folio is NULL but dereferenced. > > > folio = __filemap_get_folio(mapping, index, fgp_flags, gfp); > > > - if ((fgp_flags & FGP_HEAD) || !folio || xa_is_value(folio)) > > > + if (!folio) > > > + return NULL; > > > + if ((fgp_flags & FGP_HEAD) || xa_is_value(folio)) > > > return &folio->page; > > > > That doesn't dereference the folio. Coccicheck is wrong. > > Doing > > return &(0->page); > > is a rather obscure way of doing `return NULL;'. > > I agree the patch doesn't fix anything, but it results in saner-looking code? I suppose that's in the eye of the beholder? The original code makes more sense to me. Besides, it's in the folio-compat file; nobody should be looking at that except to figure out "What function should I be transitioning to?"
diff --git a/mm/folio-compat.c b/mm/folio-compat.c index 46fa179e32fb..54b17c1246c4 100644 --- a/mm/folio-compat.c +++ b/mm/folio-compat.c @@ -124,7 +124,9 @@ struct page *pagecache_get_page(struct address_space *mapping, pgoff_t index, struct folio *folio; folio = __filemap_get_folio(mapping, index, fgp_flags, gfp); - if ((fgp_flags & FGP_HEAD) || !folio || xa_is_value(folio)) + if (!folio) + return NULL; + if ((fgp_flags & FGP_HEAD) || xa_is_value(folio)) return &folio->page; return folio_file_page(folio, index); }
Fix following coccicheck error: mm/folio-compat.c:128:17-21: ERROR: folio is NULL but dereferenced. 'folio' would be NULL when getting a reference to a folio failed. Add a NULL check before dereferring 'folio'. Fixes: 3f0c6a07fee6 ("mm/filemap: Add filemap_get_folio") Signed-off-by: Wan Jiabing <wanjiabing@vivo.com> --- mm/folio-compat.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)