Linux-mm Archive on lore.kernel.org
 help / color / Atom feed
From: Michal Hocko <mhocko@suse.com>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: linux-mm@kvack.org, Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>,
	Mike Kravetz <mike.kravetz@oracle.com>
Subject: Re: [bug report] hugetlb, mempolicy: fix the mbind hugetlb migration
Date: Wed, 10 Jan 2018 11:47:12 +0100
Message-ID: <20180110104712.GR1732@dhcp22.suse.cz> (raw)
In-Reply-To: <20180109200539.g7chrnzftxyn3nom@mwanda>

[CC Mike and Naoya]

On Tue 09-01-18 23:05:39, Dan Carpenter wrote:
> Hello Michal Hocko,
> 
> This is a semi-automatic email about new static checker warnings.
> 
> The patch ef2fc869a863: "hugetlb, mempolicy: fix the mbind hugetlb 
> migration" from Jan 5, 2018, leads to the following Smatch complaint:
> 
>     mm/mempolicy.c:1100 new_page()
>     error: we previously assumed 'vma' could be null (see line 1092)
> 
> mm/mempolicy.c
>   1091		vma = find_vma(current->mm, start);
>   1092		while (vma) {
>                        ^^^
> There is a check for NULL here
> 
>   1093			address = page_address_in_vma(page, vma);
>   1094			if (address != -EFAULT)
>   1095				break;
>   1096			vma = vma->vm_next;
>   1097		}
>   1098	
>   1099		if (PageHuge(page)) {
>   1100			return alloc_huge_page_vma(vma, address);
>                                                    ^^^
> The patch adds a new unchecked dereference.  It might be OK?  I don't
> know.
> 
>   1101		} else if (PageTransHuge(page)) {
>   1102			struct page *thp;

Smatch is correct that the code is fishy. The patch you have outlined is
the last one to touch that area but it hasn't changed the vma logic.
It removed the BUG_ON which papepered over null VMA for your checker
previously I guess.

The THP path simply falls back to the default mem policy if vma is NULL.
We should do the same here. The patch below should do the trick.

Thanks for the report!

  reply index

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-09 20:05 Dan Carpenter
2018-01-10 10:47 ` Michal Hocko [this message]
2018-01-17 12:18   ` Michal Hocko
2018-01-17 23:15     ` Naoya Horiguchi
2018-01-19  8:47       ` Michal Hocko

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180110104712.GR1732@dhcp22.suse.cz \
    --to=mhocko@suse.com \
    --cc=dan.carpenter@oracle.com \
    --cc=linux-mm@kvack.org \
    --cc=mike.kravetz@oracle.com \
    --cc=n-horiguchi@ah.jp.nec.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-mm Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-mm/0 linux-mm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-mm linux-mm/ https://lore.kernel.org/linux-mm \
		linux-mm@kvack.org
	public-inbox-index linux-mm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kvack.linux-mm


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git