LKML Archive on
help / color / mirror / Atom feed
* Re: [PATCH linux-next] mm/folio-compat.c: folio should not be NULL when it is referenced
       [not found] <>
@ 2021-08-23  1:05 ` Matthew Wilcox
  0 siblings, 0 replies; only message in thread
From: Matthew Wilcox @ 2021-08-23  1:05 UTC (permalink / raw)
  To: Yi Wang
  Cc: akpm, linux-mm, linux-kernel, xue.zhihong, jiang.xuexin, zealci,
	Changcheng Deng

On Mon, Aug 23, 2021 at 08:47:35AM +0800, Yi Wang wrote:
> From: Changcheng Deng <>
> A bug was found by coccinelle:
> folio is NULL but dereferenced
> Therefore,added a check to make sure 'folio' is not NULL.
> Reported-by: Zeal Robot <>

Your robot is overzealous.  This does not dereference folio; rather it
takes the address of the page element of the folio structure.

By a strict reading of the C spec, it is not allowed.  However, GCC
(and I assume Clang) does the right thing.

>  	folio = __filemap_get_folio(mapping, index, fgp_flags, gfp);
> -	if ((fgp_flags & FGP_HEAD) || !folio || xa_is_value(folio))
> -		return &folio->page;
> +	if ((fgp_flags & FGP_HEAD) || xa_is_value(folio))
> +		if (folio != NULL)
> +			return &folio->page;
>  	return folio_file_page(folio, index);

This is definitely wrong.  Did you test it?  I bet you get a NULL
pointer dereference if you try it.

You could potentially make the case for:

	if (!folio)
		return NULL;
	if ((fgp_flags & FGP_HEAD) || xa_is_value(folio))
		return &folio->page;

but you actually have the same problem with the C spec, that unless
folio is actually a pointer to a folio, then &folio->page is
_technically_ undefined.  So it would have to be something even
more complex to be pedantically correct.

It's just not worth it.  Fix your tool.

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2021-08-23  1:06 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <>
2021-08-23  1:05 ` [PATCH linux-next] mm/folio-compat.c: folio should not be NULL when it is referenced Matthew Wilcox

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).