linux-kernel - Re: Regression after commit 19809c2da28a ("mm, vmalloc: use __GFP

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180208130649.GA15846@bombadil.infradead.org>
Date:   Thu, 8 Feb 2018 05:06:49 -0800
From:   Matthew Wilcox <willy@...radead.org>
To:     Kai Heng Feng <kai.heng.feng@...onical.com>
Cc:     Michal Hocko <mhocko@...e.com>, Laura Abbott <labbott@...hat.com>,
        linux-mm@...ck.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: Regression after commit 19809c2da28a ("mm, vmalloc: use
 __GFP_HIGHMEM implicitly")

On Thu, Feb 08, 2018 at 02:29:57PM +0800, Kai Heng Feng wrote:
> A user with i386 instead of AMD64 machine reports [1] that commit 19809c2da28a ("mm, vmalloc: use __GFP_HIGHMEM implicitly”) causes a regression.
> BUG_ON(PageHighMem(pg)) in drivers/media/common/saa7146/saa7146_core.c always gets triggered after that commit.

Well, the BUG_ON is wrong.  You can absolutely have pages which are both
HighMem and under the 4GB boundary.  Only the first 896MB (iirc) are LowMem,
and the next 3GB of pages are available to vmalloc_32().

> Also there are other BUG_ON(PageHighMem()) in drivers/media, I think they will get hit by same regression in 32bit machine too.

I fixed one of them.  I think the other three are also bogus, but it's
hard to say; the comments say "DMA to HighMem might not work", and they
probably mean "above the 4GB boundary", but I really don't know.

(since two drivers now have this code, maybe it should be part of the core
MM API?  Or maybe there's already something better they should be using?)

diff --git a/drivers/media/common/saa7146/saa7146_core.c b/drivers/media/common/saa7146/saa7146_core.c
index 9f7c5b0a6b45..329fd43228ff 100644
--- a/drivers/media/common/saa7146/saa7146_core.c
+++ b/drivers/media/common/saa7146/saa7146_core.c
@@ -160,7 +160,7 @@ static struct scatterlist* vmalloc_to_sg(unsigned char *virt, int nr_pages)
 		pg = vmalloc_to_page(virt);
 		if (NULL == pg)
 			goto err;
-		BUG_ON(PageHighMem(pg));
+		BUG_ON(page_to_pfn(pg) >= (1 << (32 - PAGE_SHIFT)));
 		sg_set_page(&sglist[i], pg, PAGE_SIZE, 0);
 	}
 	return sglist;
diff --git a/drivers/media/v4l2-core/videobuf-dma-sg.c b/drivers/media/v4l2-core/videobuf-dma-sg.c
index f412429cf5ba..b5ec74b9c867 100644
--- a/drivers/media/v4l2-core/videobuf-dma-sg.c
+++ b/drivers/media/v4l2-core/videobuf-dma-sg.c
@@ -77,7 +77,7 @@ static struct scatterlist *videobuf_vmalloc_to_sg(unsigned char *virt,
 		pg = vmalloc_to_page(virt);
 		if (NULL == pg)
 			goto err;
-		BUG_ON(PageHighMem(pg));
+		BUG_ON(page_to_pfn(pg) >= (1 << (32 - PAGE_SHIFT)));
 		sg_set_page(&sglist[i], pg, PAGE_SIZE, 0);
 	}
 	return sglist;