From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.5 required=3.0 tests=MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 68F4DC33C99 for ; Fri, 10 Jan 2020 06:23:00 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 261D82072E for ; Fri, 10 Jan 2020 06:23:00 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 261D82072E Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id AD68E8E0005; Fri, 10 Jan 2020 01:22:59 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id A86F88E0001; Fri, 10 Jan 2020 01:22:59 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 99C468E0005; Fri, 10 Jan 2020 01:22:59 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0144.hostedemail.com [216.40.44.144]) by kanga.kvack.org (Postfix) with ESMTP id 843E78E0001 for ; Fri, 10 Jan 2020 01:22:59 -0500 (EST) Received: from smtpin15.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with SMTP id 3F21E4DA4 for ; Fri, 10 Jan 2020 06:22:59 +0000 (UTC) X-FDA: 76360731678.15.floor85_371fe9fc05a36 X-HE-Tag: floor85_371fe9fc05a36 X-Filterd-Recvd-Size: 5417 Received: from mail-wm1-f66.google.com (mail-wm1-f66.google.com [209.85.128.66]) by imf10.hostedemail.com (Postfix) with ESMTP for ; Fri, 10 Jan 2020 06:22:58 +0000 (UTC) Received: by mail-wm1-f66.google.com with SMTP id d73so713064wmd.1 for ; Thu, 09 Jan 2020 22:22:58 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=wZqPLKKomb4mvUH5lDRqCDOHw7Mr9KMVkg7T95nqkSw=; b=AqPQHtSCt2RGNIaIDWAbOb0p8Ju7Zv0Fh2WnCA+9uWEYBeJ54PdUmwVZz82n1n5WpJ fWXD4OddILQ93gcsqokgdEaXubpkIz0utDgO5y7OWM4L9zwHWyYI6coratw4XvNGmCl7 xyRHbI9qF2tTINgyYfl7MgTk2uTqDBtA0iTuKckeTa8EDNsyyaANEwkcQRXekWBayf0m 2bGgf1ib/g/UWgf6Vhc712Nh7voug03SCr/9xP1Oyo/VpzfVDhBAUHNSYU3gAwQooMxR s66xmQGRLpjMX1PySF/Y+8g8hsTGxlLxg3taO3zyrF2LWxiogkJ6x9YKn/a2tAWWL1vu Di7g== X-Gm-Message-State: APjAAAX89SiFqdnlMcc9Z19+FODjto6OApOjyFZYZ2OaovEsqaHKlEqO vLBJizQkrU45HWlSs3sk9uY= X-Google-Smtp-Source: APXvYqwcAizoBGCm05eTYA5ZdogpnZBE12mO/rFvhk0VI0akOVuBJZY5tlKslX6pEXVol092u5a+1Q== X-Received: by 2002:a7b:c7d4:: with SMTP id z20mr2256835wmk.42.1578637377530; Thu, 09 Jan 2020 22:22:57 -0800 (PST) Received: from localhost (ip-37-188-146-105.eurotel.cz. [37.188.146.105]) by smtp.gmail.com with ESMTPSA id u7sm1059268wmj.3.2020.01.09.22.22.56 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 09 Jan 2020 22:22:56 -0800 (PST) Date: Fri, 10 Jan 2020 07:22:55 +0100 From: Michal Hocko To: Li Xinhai Cc: Mike Kravetz , "linux-mm@kvack.org" , "kirill.shutemov" Subject: Re: [PATCH v2] mm/page_vma_mapped.c: Detect mismatched pfn of hugetlbfs page in pfn_in_hpage() Message-ID: <20200110062255.GA29802@dhcp22.suse.cz> References: <1578579963-6075-1-git-send-email-lixinhai.lxh@gmail.com> <20200109150052.GT4951@dhcp22.suse.cz> <568a2f44-ad16-f3b5-4691-4e6f6ff21bf1@oracle.com> <2020011006480540632892@gmail.com> <20200110105254780538125@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <20200110105254780538125@gmail.com> User-Agent: Mutt/1.12.2 (2019-09-21) Content-Transfer-Encoding: quoted-printable X-Bogosity: Ham, tests=bogofilter, spamicity=0.000001, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Fri 10-01-20 10:52:56, Li Xinhai wrote: > On 2020-01-10=A0at 07:00=A0Mike Kravetz=A0wrote: > >On 1/9/20 2:48 PM, Li Xinhai wrote: > >> oops, I didn't write the code correctly. I should wrote it as > >> > >> if (pfn >=3D hpage_pfn && pfn - hpage_pfn < hpage_nr_pages(hpage)) { > >> VM_BUG_ON_PAGE(PageHuge(hpage) && pfn !=3D hpage_pfn, hpage); > >> return true; > >> } > >> > >> return false; > >> > >> hpage_nr_pages(hpage) give us HPAGE_PMD_NR for THP and hugetlbfs pag= e, > >> but remapping PTE to a differrnt hugetlbfs page still allowed, so pu= t the BUG code > >> into this condition is necessary. By this way, if it was not a exact= match for PageHuge, > >> then it is a bug. > > > >Thank you.=A0 I think we all agree on what the proposed code is doing. > >However, we would like to know why you believe this code should be add= ed. > >For example, > >- Did you actually encounter this situation (PageHuge(hpage) && pfn !=3D > >=A0 hpage_pfn)? > >- Did you discover some code path where we are likely to encounter thi= s > >=A0 situation? > >- Some other reason?=20 >=20 > I didn't actually encounter this condition. >=20 > There are two ways for faulty code, > 1. one is from the 'hpage', it could be head or tail page of hugetlbfs = (I see that > current=A0code=A0make sure always call with head page as you mentioned)= . Luckly, > we=A0catch the tail page case as BUG at begining of this mapped_walk(th= e > page_hstate(page) return NULL for tail page). > 2. The other is from the content stored in the PTE, wihch we used as 'p= fn' and > compare with 'hpage'. >=20 > Current code matches 'pfn' and 'hpage' like below: > - normal 4k page: hpage_pfn <=3D pfn < hpage_pfn + 1 > - THP, hugetlbfs page: =A0hpage_pfn <=3D pfn < hpage_pfn +=A0HPAGE_PMD_= NR > we need do exact match for normal 4K page and hugetlbfs page, and range > match for THP. This still doesn't really explain why to add the BUG_ON, I am afraid. pfn_in_hpage is called from the vma walk. check_pte is reponsible to check the page table mapping so the input to pfn_in_hpage should be already sanitized. If it is not then that should be fixed and {VM_}BUG_ON is not the best way to do such a sanitization IMHO. First of all this is all under locks so crashing would likely mean a follow up problems. On the other hand a failure can be handled gracefully AFAICS. That being said I still do not see how this is going to help with anything. Please note that adding {VM}_BUG_ON as general asserts is strongly discouraged. Those should be added only when there is a data corruption detected (and then it should likely be BUG_ON rather than VM_BUG_ON) that cannot be handled gracefully or when it considerably improves debugability of very subtle problems. --=20 Michal Hocko SUSE Labs