Skip to content

Commit a4866aa

Browse files
committed
mm: Tighten x86 /dev/mem with zeroing reads
Under CONFIG_STRICT_DEVMEM, reading System RAM through /dev/mem is disallowed. However, on x86, the first 1MB was always allowed for BIOS and similar things, regardless of it actually being System RAM. It was possible for heap to end up getting allocated in low 1MB RAM, and then read by things like x86info or dd, which would trip hardened usercopy: usercopy: kernel memory exposure attempt detected from ffff880000090000 (dma-kmalloc-256) (4096 bytes) This changes the x86 exception for the low 1MB by reading back zeros for System RAM areas instead of blindly allowing them. More work is needed to extend this to mmap, but currently mmap doesn't go through usercopy, so hardened usercopy won't Oops the kernel. Reported-by: Tommi Rantala <tommi.t.rantala@nokia.com> Tested-by: Tommi Rantala <tommi.t.rantala@nokia.com> Signed-off-by: Kees Cook <keescook@chromium.org>
1 parent b9b3322 commit a4866aa

File tree

2 files changed

+82
-41
lines changed

2 files changed

+82
-41
lines changed

Diff for: arch/x86/mm/init.c

+30-11
Original file line numberDiff line numberDiff line change
@@ -643,21 +643,40 @@ void __init init_mem_mapping(void)
643643
* devmem_is_allowed() checks to see if /dev/mem access to a certain address
644644
* is valid. The argument is a physical page number.
645645
*
646-
*
647-
* On x86, access has to be given to the first megabyte of ram because that area
648-
* contains BIOS code and data regions used by X and dosemu and similar apps.
649-
* Access has to be given to non-kernel-ram areas as well, these contain the PCI
650-
* mmio resources as well as potential bios/acpi data regions.
646+
* On x86, access has to be given to the first megabyte of RAM because that
647+
* area traditionally contains BIOS code and data regions used by X, dosemu,
648+
* and similar apps. Since they map the entire memory range, the whole range
649+
* must be allowed (for mapping), but any areas that would otherwise be
650+
* disallowed are flagged as being "zero filled" instead of rejected.
651+
* Access has to be given to non-kernel-ram areas as well, these contain the
652+
* PCI mmio resources as well as potential bios/acpi data regions.
651653
*/
652654
int devmem_is_allowed(unsigned long pagenr)
653655
{
654-
if (pagenr < 256)
655-
return 1;
656-
if (iomem_is_exclusive(pagenr << PAGE_SHIFT))
656+
if (page_is_ram(pagenr)) {
657+
/*
658+
* For disallowed memory regions in the low 1MB range,
659+
* request that the page be shown as all zeros.
660+
*/
661+
if (pagenr < 256)
662+
return 2;
663+
664+
return 0;
665+
}
666+
667+
/*
668+
* This must follow RAM test, since System RAM is considered a
669+
* restricted resource under CONFIG_STRICT_IOMEM.
670+
*/
671+
if (iomem_is_exclusive(pagenr << PAGE_SHIFT)) {
672+
/* Low 1MB bypasses iomem restrictions. */
673+
if (pagenr < 256)
674+
return 1;
675+
657676
return 0;
658-
if (!page_is_ram(pagenr))
659-
return 1;
660-
return 0;
677+
}
678+
679+
return 1;
661680
}
662681

663682
void free_init_pages(char *what, unsigned long begin, unsigned long end)

Diff for: drivers/char/mem.c

+52-30
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,10 @@ static inline int valid_mmap_phys_addr_range(unsigned long pfn, size_t size)
6060
#endif
6161

6262
#ifdef CONFIG_STRICT_DEVMEM
63+
static inline int page_is_allowed(unsigned long pfn)
64+
{
65+
return devmem_is_allowed(pfn);
66+
}
6367
static inline int range_is_allowed(unsigned long pfn, unsigned long size)
6468
{
6569
u64 from = ((u64)pfn) << PAGE_SHIFT;
@@ -75,6 +79,10 @@ static inline int range_is_allowed(unsigned long pfn, unsigned long size)
7579
return 1;
7680
}
7781
#else
82+
static inline int page_is_allowed(unsigned long pfn)
83+
{
84+
return 1;
85+
}
7886
static inline int range_is_allowed(unsigned long pfn, unsigned long size)
7987
{
8088
return 1;
@@ -122,23 +130,31 @@ static ssize_t read_mem(struct file *file, char __user *buf,
122130

123131
while (count > 0) {
124132
unsigned long remaining;
133+
int allowed;
125134

126135
sz = size_inside_page(p, count);
127136

128-
if (!range_is_allowed(p >> PAGE_SHIFT, count))
137+
allowed = page_is_allowed(p >> PAGE_SHIFT);
138+
if (!allowed)
129139
return -EPERM;
140+
if (allowed == 2) {
141+
/* Show zeros for restricted memory. */
142+
remaining = clear_user(buf, sz);
143+
} else {
144+
/*
145+
* On ia64 if a page has been mapped somewhere as
146+
* uncached, then it must also be accessed uncached
147+
* by the kernel or data corruption may occur.
148+
*/
149+
ptr = xlate_dev_mem_ptr(p);
150+
if (!ptr)
151+
return -EFAULT;
130152

131-
/*
132-
* On ia64 if a page has been mapped somewhere as uncached, then
133-
* it must also be accessed uncached by the kernel or data
134-
* corruption may occur.
135-
*/
136-
ptr = xlate_dev_mem_ptr(p);
137-
if (!ptr)
138-
return -EFAULT;
153+
remaining = copy_to_user(buf, ptr, sz);
154+
155+
unxlate_dev_mem_ptr(p, ptr);
156+
}
139157

140-
remaining = copy_to_user(buf, ptr, sz);
141-
unxlate_dev_mem_ptr(p, ptr);
142158
if (remaining)
143159
return -EFAULT;
144160

@@ -181,30 +197,36 @@ static ssize_t write_mem(struct file *file, const char __user *buf,
181197
#endif
182198

183199
while (count > 0) {
200+
int allowed;
201+
184202
sz = size_inside_page(p, count);
185203

186-
if (!range_is_allowed(p >> PAGE_SHIFT, sz))
204+
allowed = page_is_allowed(p >> PAGE_SHIFT);
205+
if (!allowed)
187206
return -EPERM;
188207

189-
/*
190-
* On ia64 if a page has been mapped somewhere as uncached, then
191-
* it must also be accessed uncached by the kernel or data
192-
* corruption may occur.
193-
*/
194-
ptr = xlate_dev_mem_ptr(p);
195-
if (!ptr) {
196-
if (written)
197-
break;
198-
return -EFAULT;
199-
}
208+
/* Skip actual writing when a page is marked as restricted. */
209+
if (allowed == 1) {
210+
/*
211+
* On ia64 if a page has been mapped somewhere as
212+
* uncached, then it must also be accessed uncached
213+
* by the kernel or data corruption may occur.
214+
*/
215+
ptr = xlate_dev_mem_ptr(p);
216+
if (!ptr) {
217+
if (written)
218+
break;
219+
return -EFAULT;
220+
}
200221

201-
copied = copy_from_user(ptr, buf, sz);
202-
unxlate_dev_mem_ptr(p, ptr);
203-
if (copied) {
204-
written += sz - copied;
205-
if (written)
206-
break;
207-
return -EFAULT;
222+
copied = copy_from_user(ptr, buf, sz);
223+
unxlate_dev_mem_ptr(p, ptr);
224+
if (copied) {
225+
written += sz - copied;
226+
if (written)
227+
break;
228+
return -EFAULT;
229+
}
208230
}
209231

210232
buf += sz;

0 commit comments

Comments
 (0)