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 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2E888C46CA1 for ; Mon, 16 Oct 2023 03:28:34 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id A31FC8D002C; Sun, 15 Oct 2023 23:28:33 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 9BAAA8D0001; Sun, 15 Oct 2023 23:28:33 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 85BC88D002C; Sun, 15 Oct 2023 23:28:33 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 740E48D0001 for ; Sun, 15 Oct 2023 23:28:33 -0400 (EDT) Received: from smtpin15.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id D18C8160A90 for ; Mon, 16 Oct 2023 03:28:32 +0000 (UTC) X-FDA: 81349892064.15.6CA7C69 Received: from mail-yb1-f178.google.com (mail-yb1-f178.google.com [209.85.219.178]) by imf19.hostedemail.com (Postfix) with ESMTP id 2144C1A0007 for ; Mon, 16 Oct 2023 03:28:30 +0000 (UTC) Authentication-Results: imf19.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=GfQQVqCT; spf=pass (imf19.hostedemail.com: domain of vishal.moola@gmail.com designates 209.85.219.178 as permitted sender) smtp.mailfrom=vishal.moola@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1697426911; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=drO1mP31WiAmio3P3yCrOfvtL3Fjrd3FB9JD0sPIXRA=; b=G7t5sfyhnE7Ju4nxl+1iK4qyjRjYFmSfAy9fdRRXUY5QKRISKh4G8WWrsl9sBLDfQyr4AU cfmj23sV/B+n+7KRYm/Fh7tC90v9yxXq56ywR81ge+ITwNMiOnrCu6SV0z7jbBF8+prcQ5 pl+4aeockGV9K/D+Bac6vhBEka/Z61U= ARC-Authentication-Results: i=1; imf19.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=GfQQVqCT; spf=pass (imf19.hostedemail.com: domain of vishal.moola@gmail.com designates 209.85.219.178 as permitted sender) smtp.mailfrom=vishal.moola@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1697426911; a=rsa-sha256; cv=none; b=XsOi+f+Rd8FTg9uHoQLriAuqMYWOChgJzpebZihFy3uHgJJULeYD/DxuXaV11klNo3ngbf /P+Lu4MOyeybfcAY3jMgZPq52/kmG8/I2r+3MfwKhMPd8ySOObK90R8lLQB8/9hkHcuAT3 +FDBqtpA9o8t6QiELGg8EeWk9xeHzBA= Received: by mail-yb1-f178.google.com with SMTP id 3f1490d57ef6-d81d09d883dso4512625276.0 for ; Sun, 15 Oct 2023 20:28:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1697426910; x=1698031710; darn=kvack.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=drO1mP31WiAmio3P3yCrOfvtL3Fjrd3FB9JD0sPIXRA=; b=GfQQVqCTysVAYoM9vS9zr85enl+AR3q4SplZcf4UP88Ie//rHjJvSFlw2UCSjoE6ib 6C4R6093e7XqOoozdqFIhhKfk/5+yqQmoqLBC5lysH3s1ahgt9F5Yteyr+EA8G7iTr4q 7lr+zO4j2ZrQHN75Y1/GXA6ZAgYzVOLX+MzJSJcxcqGFd6RAtmRPYBPQN4dtEckqo042 2LXEf6abSEngXpsLV/tQrvMkybx/K9/P7djwmbvb4/Jw1FJXcwyyN4TzdbjhFcy1AVGG LQ++M1wcU9Qidkc2KK/CBLCgf6/uJb0rs1EpyYz1ISrvplx2KKbQ+YcE7Vg6KNOGwa6X 4wNQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1697426910; x=1698031710; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=drO1mP31WiAmio3P3yCrOfvtL3Fjrd3FB9JD0sPIXRA=; b=niU3zSaMOb7jxxOnzcpHqb7TiK5n5ZW/WxRXROjKAo52JDCtMHIr9B8eHO3cyBVQGs UAgS/cFEDhygW+jfpw5p60N6/MSXc61NhiV9k//+sDzluPaXr9tzCrVU7KgaqE40dSNP 27hfLCgkmmwPQdLIoH6wlHkcS+NZqMtR3Ly5CSJqobVBvUl9Dz1xmv+czGJc10weKVXy czQTIDHZU9RSo392LdMnjejt6vYRjm+hJt+XxwKDnel5eiBmxK9apiLIliO96MSUprgA A8iC7VuvuuWq5oe+Ek9tZtHskijghliZ1GdY3FKs9OM4pIXPhVFFpJjTadYmgR9W0vlu eDIQ== X-Gm-Message-State: AOJu0YzgdOehmwxF512QuvWs1q4UCNs086SmEJfxIWoPIU4cyy8oPE5W 3b14NEO2vkjy8egUf130p5NDEmOB2T6lYuJbmKA= X-Google-Smtp-Source: AGHT+IGYr80MQnOGl5txzOgiPVkU6G+nY1pZViCMjmVqceQ+hQ3pvycjUNbc/RuqThsMzHs0kosmPalFKpTN0SFtdEM= X-Received: by 2002:a25:df11:0:b0:d9a:5ab0:cb01 with SMTP id w17-20020a25df11000000b00d9a5ab0cb01mr16802288ybg.27.1697426910190; Sun, 15 Oct 2023 20:28:30 -0700 (PDT) MIME-Version: 1.0 References: <20231012011106.2425309-1-hyesoo.yu@samsung.com> <20231016003200.GA445850@tiffany> In-Reply-To: <20231016003200.GA445850@tiffany> From: Vishal Moola Date: Sun, 15 Oct 2023 20:28:18 -0700 Message-ID: Subject: Re: [PATCH] mm: page_alloc: check the order of compound page event when the order is 0 To: Hyesoo Yu Cc: Andrew Morton , linux-mm@kvack.org, linux-kernel@vger.kernel.org Content-Type: multipart/alternative; boundary="00000000000003a1240607cd0078" X-Rspamd-Queue-Id: 2144C1A0007 X-Rspam-User: X-Stat-Signature: mwdb9uoyis568nhnh16741q1be5kam5p X-Rspamd-Server: rspam01 X-HE-Tag: 1697426910-724511 X-HE-Meta: U2FsdGVkX19HJJh00yrUkUPt9BoDoh+/5KM0tsxtY+HU2ujZ9ZyIu8stR/vE6qvbRiWZaLvoF6BHH8/su9fEefd3nrdm5Mum0tcwqau38zd4hPIjNuD3L17I+zN45BYS+KzcKOdjGpi/SGa0tGSASXJjJ2FWo4tLInLJd8EnKQZ6ubvCrEEcqQIb8kgjhxkNwSXvIYukfRvMy6OXQ4X8iaZgLxnmmHluWWlQoXN7y6XXDJeNU7jUqvBbyXThjX4yHBySYt5dqQUFdKl2cjHs86FA997+YtXE44FHk5YcJuW67GG0zz0VJX/U/So6VdrDjJZURuhtFhWHj9kd30EtsaVO7+knT51x/3C2l4h0+Pvw7dKI6XRVWa/oLe3LcgJcrvG0XwPnZ8CV8S7TV3eox69XaIGNOqpmKi+5CqqcMoUPa74BvVO1wd0wNP4auJp26l3yJEzQLKcbPm5BWOJZbdc+z/Mx7cLJDn5jUv3g1qe0SNl7CbWaJLGafLVR3ONCpHXkcmdqbsB6aiB37VHJZMRsQuVWBUAqxI3SMwk/eprhKPwusC1qmB77lXosBsXlrh8MMvPkD7Hs2Kcvuux593Q9pm6G+r8FtXHT6bQIMHTzPyeuHVcU8gcUv9u1RJVrOfBxG4Bo6den7Oq7JXe2GZos7yE0MqNp2x/LTXHFgbVaSnmhxO44Xyt4QhSIDQBx+et9pOjvrQo7vrL9xeih9ivNzi5tIoLPZFMVGzKhlXJClPoUVgaeG90MEq2LpGApWdl1hnD2j3PyDAR1j4L3CXqf1433TWBD/7ICsaThiwdW2mrGkRlvWZ6r/qhWLqhGuSrT/KVnemPbfviuBH8513yohE91QLudUhfrFPIfm6cNWlUImU6vpCAb2U0LDHCeUZ2kg0cr+zST9jI0GPybrp9pNmoxcYVHVeSR82H7OY+je4T+85XuJad+c7uSMU2pEGY1hQVr40oVsofOydt +KUkR+YP FrqShTdAPgqERv3Bvl2/cE0OzcKrxvO0ch/hgGBZYsKJaeQLwp/PIfoVgZIj3AlLR++QqSx6q1w6H7IXT6QIa0MqWt5Ha5//Arz7D6ZFtUV4yDR8V0+Y8H++ZSdo9pOTo+HIrBST1tzO5x+DYh7Wi2S6ofrHDGsNcZFQ7hDAP8MdPZpudVUzAhoabt2BwHJLGGSOGweEP5mpuU7+JnCEkDZf2TSM81Jr6MFBNNB8pjgLzHNctM7zGBFuR1BXfvpFieQajaCCD/4NjhH1AZuOkHS0THxDJrg9gD+tWHFSLqA5glQvXlRpThL6jtvHHHIrb1EiMgnN0djJUNBaLl1dFcP7rE/Zmtxtp93haD6cGEYgc6MX5npBSQz9VqrXtX+DtGSidHQ0euC/8n8EL6kpMQ5OvSkb8wdFeHWwdEHj6u+rBFrSMqfgjoY7/69aE5uvjEPzINKtsZZF6okAFyAonVuXEssbpDaIDEckNM1XdtU3aTKbZtylWcStKmykRfwwskPmJG0vvuRvIwo1hRA0eqt1sttxdrTcOIPd1psGihrWARzeKZNJclURbJzpHsUvA70GlXnxwRCoE2mXZgpXxlBPtxEFAv1J07Kn1 X-Bogosity: Ham, tests=bogofilter, spamicity=0.020468, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: --00000000000003a1240607cd0078 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Sun, Oct 15, 2023 at 5:42=E2=80=AFPM Hyesoo Yu w= rote: > > On Fri, Oct 13, 2023 at 01:54:08PM -0700, Vishal Moola wrote: > > On Thu, Oct 12, 2023 at 10:11:06AM +0900, Hyesoo Yu wrote: > > > For compound pages, the head sets the PG_head flag and > > > the tail sets the compound_head to indicate the head page. > > > If a user allocates a compound page and frees it with a different > > > order, the compound page information will not be properly > > > initialized. To detect this problem, compound_page(page) and s/compound_page/compound_order/ > > > the order are compared, but it is not checked when the order is 0. > > > That error should be checked regardless of the order. With this many mentions of "the order", it is easy to misinterpret "the order" to be referencing the page order rather than the order of pages we are trying to free. I recommend replacing "the order" with "the order argument" or something similar for clarity. > > I believe all compound pages are order >=3D 1, so this error can't occu= r > > when the order is 0. > > > > Yes. All compound pages are order >=3D 1. > However if the user uses the API incorrectly, the order value could be zero. I see, thanks for clarifying that. With the commit message changes above: Reviewed-by: Vishal Moola (Oracle) > For example, > > addr =3D alloc_pages(GFP_COMP, 2); > free_pages(addr, 0); > > (struct page[16])0xFFFFFFFE21715100 =3D ( > (flags =3D 0x4000000000000200, lru =3D (next =3D 0x0, prev =3D 0xDEAD000000000122),// Clear PG_head > (flags =3D 0x4000000000000000, lru =3D (next =3D 0xFFFFFFFE21715101, prev= =3D 0xFFFFFFFF00000201), // Remain compound head > > It is memory leak, and it also makes system stability problem. > on isolation_single_pageblock, That case makes infinite loops. > > for (pfn =3D start_pfn; pfn < boundary_pfn; ) { > if (PageCompound(page)) { // page[1] is compound page > struct page *head =3D compound_head(page); // page[0] > unsigned long head_pfn =3D page_to_pfn(head); > unsigned long nr_pages =3D compound_nr(head); // nr_pages is 1 since page[0] is not compound page. > > if (head_pfn + nr_pages <=3D boundary_pfn) { > pfn =3D head_pfn + nr_pages; // pfn is set as page[1]. > continue; > } > } > > So, I guess, we have to check the incorrect use in free_pages_prepare. > > Thanks, > Hyesoo Yu. --00000000000003a1240607cd0078 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


On Sun, Oct 15, 2023 at 5:42=E2=80=AFPM Hyesoo Yu = <hyesoo.yu@samsung.com> = wrote:
>
> On Fri, Oct 13, 2023 at 01:54:08PM -0700, Vishal Moo= la wrote:
> > On Thu, Oct 12, 2023 at 10:11:06AM +0900, Hyesoo Yu = wrote:
> > > For compound pages, the head sets the PG_head flag= and
> > > the tail sets the compound_head to indicate the head= page.
> > > If a user allocates a compound page and frees it w= ith a different
> > > order, the compound page information will= not be properly
> > > initialized. To detect this problem, com= pound_page(page) and

s/compound_page/compound_order/

> > > the order are compared, but it is not checked whe= n the order is 0.
> > > That error should be checked reg= ardless of the order.

With this many mentions of "the order&quo= t;, it is easy to misinterpret "the order"
to be refere= ncing the page order rather than the order of pages we are trying
to free. I recommend replacing "the order" with "the order = argument" or
something similar for clarity.

&g= t; > I believe all compound pages are order >=3D 1, so this error can= 't occur
> > when the order is 0.
> >
>
>= Yes. All compound pages are order >=3D 1.
> However if the user u= ses the API incorrectly, the order value could be zero.

I see, thank= s for clarifying that.

With the commit messag= e changes above:
Reviewed-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>

> For example,
>
> addr =3D alloc_pages(GFP_COMP, 2);> free_pages(addr, 0);
>
> (struct page[16])0xFFFFFFFE2171= 5100 =3D (
> (flags =3D 0x4000000000000200, lru =3D (next =3D 0x0, pr= ev =3D 0xDEAD000000000122),// =C2=A0Clear PG_head
> (flags =3D 0x4000= 000000000000, lru =3D (next =3D 0xFFFFFFFE21715101, prev =3D 0xFFFFFFFF0000= 0201), =C2=A0// Remain compound head
>
> It is memory leak, and= it also makes system stability problem.
> on isolation_single_pagebl= ock, That case makes infinite loops.
>
> for (pfn =3D start_pfn= ; pfn < boundary_pfn; ) {
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (PageCo= mpound(page)) { // page[1] is compound page
> =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 struct page *head =3D compound_head(page= ); // page[0]
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 unsigned long head_pfn =3D page_to_pfn(head);
> =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 unsigned long nr_pages =3D compound_= nr(head); // nr_pages is 1 since page[0] is not compound page.
>
&= gt; =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (head_pfn + = nr_pages <=3D boundary_pfn) {
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 pfn =3D head_pfn + nr_pag= es; // pfn is set as page[1].
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 continue;
> =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 }
> }
>
>= ; So, I guess, we have to check the incorrect use in free_pages_prepare.>
> Thanks,
> Hyesoo Yu.
--00000000000003a1240607cd0078--