On 12/2/24 17:00, David Hildenbrand wrote:
On 02.12.24 16:41, William Roche wrote:
Hello David,
Hi,
sorry for reviewing yet, I was rather sick the last 1.5 weeks.
I hope you get well soon!
I've finally tested many page mapping possibilities and tried to
identify the error injection reaction on these pages to see if mmap()
can be used to recover the impacted area.
I'm using the latest upstream kernel I have for that:
6.12.0-rc7.master.20241117.ol9.x86_64
But I also got similar results with a kernel not supporting
MADV_DONTNEED, for example: 5.15.0-301.163.5.2.el9uek.x86_64
Let's start with mapping a file without modifying the mapped area:
In this case we should have a clean page cache mapped in the process.
If an error is injected on this page, the kernel doesn't even inform the
process about the error as the page is replaced (no matter if the
mapping was shared of not).
The kernel indicates this situation with the following messages:
[10759.371701] Injecting memory failure at pfn 0x10d88e
[10759.374922] Memory failure: 0x10d88e: corrupted page was clean:
dropped without side effects
[10759.377525] Memory failure: 0x10d88e: recovery action for clean LRU
page: Recovered
Right. The reason here is that we can simply allocate a new page and
load data from disk. No corruption.
Now when the page content is modified, in the case of standard page
size, we need to consider a MAP_PRIVATE or MAP_SHARED
- in the case of a MAP_PRIVATE page, this page is corrupted and the
modified data are lost, the kernel will use the SIGBUS mechanism to
inform this process if needed.
But remapping the area sweeps away the poisoned page, and allows the
process to use the area.
- In the case of a MAP_SHARED page, if the content hasn't been sync'ed
with the file backend, we also loose the modified data, and the kernel
can also raise SIGBUS.
Remapping the area recreates a page cache from the "on disk" file
content, clearing the error.
In a mmap(MAP_SHARED, fd) region that will also require fallocate IIUC.
I would have expected the same thing, but what I noticed is that in the
case of !hugetlb, even poisoned shared memory seem to be recovered with:
mmap(location, size, PROT_READ|PROT_WRITE, MAP_SHARED|MAP_FIXED, fd, 0)
But we can decide that the normal behavior in this case would be to
require an fallocate() call, and if this call fails, we fail the recovery.
My tests showed that a standard sized page can be replaced by a new one
calling the above mmap(). And shared hugetlb case doesn't work this way.
In both cases, the kernel indicates messages like:
[41589.578750] Injecting memory failure for pfn 0x122105 at process
virtual address 0x7f13bad55000
[41589.582237] Memory failure: 0x122105: Sending SIGBUS to testdh:7343
due to hardware memory corruption
[41589.584907] Memory failure: 0x122105: recovery action for dirty LRU
page: Recovered
> >
Now in the case of hugetlbfs pages:
This case behaves the same way as the standard page size when using
MAP_PRIVATE: mmap of the underlying file is able to sweep away the
poisoned page.
But the MAP_SHARED case is different: mmap() doesn't clear anything.
fallocate() must be used.
Yes, I recall that is what I initially said. The behavior with
MAP_SHARED should be consistent between hugetlb and !hugetlb.
The tests showed that they are different.
In both cases, the kernel indicates messages like:
[89141.724295] Injecting memory failure for pfn 0x117800 at process
virtual address 0x7fd148800000
[89141.727103] Memory failure: 0x117800: Sending SIGBUS to testdh:9480
due to hardware memory corruption
[89141.729829] Memory failure: 0x117800: recovery action for huge page:
Recovered
Conclusion:
We can't count on the mmap() method only for the hugetlbfs case with
MAP_SHARED.
At the end of this email, I included the source code of a simplistic
test case that shows that the page is replaced in the case of standard
page size.
The idea of this test is simple:
1/ Create a local FILE with:
# dd if=/dev/zero of=./FILE bs=4k count=2
2+0 records in
2+0 records out
8192 bytes (8.2 kB, 8.0 KiB) copied, 0.000337674 s, 24.3 MB/s
2/ As root run:
# ./poisonedShared4k
Mapping 8192 bytes from file FILE
Reading and writing the first 2 pages content:
Read: Read: Wrote: Initial mem page 0
Wrote: Initial mem page 1
Data pages at 0x7f71a19d6000 physically 0x124fb0000
Data pages at 0x7f71a19d7000 physically 0x128ce4000
Poisoning 4k at 0x7f71a19d6000
Signal 7 received
code 4 Signal code
addr 0x7f71a19d6000 Memory location
si_addr_lsb 12
siglongjmp used
Remapping the poisoned page
Reading and writing the first 2 pages content:
Read: Read: Initial mem page 1
Wrote: Rewrite mem page 0
Wrote: Rewrite mem page 1
Data pages at 0x7f71a19d6000 physically 0x10c367000
Data pages at 0x7f71a19d7000 physically 0x128ce4000
---
As we can see, this process:
- maps the FILE,
- tries to read and write the beginning of the first 2 pages
- gives their physical addresses
- poison the first page with a madvise(MADV_HWPOISON) call
- shows the SIGBUS signal received and recovers from it
- simply remaps the same page from the file
- tries again to read and write the beginning of the first 2 pages
- gives their physical addresses
---
The test (run on 6.12.0-rc7.master.20241117.ol9.x86_64) showed the
memory is usable after the remap.
Do you see a different behavior, with an even more recent kernel ?
>> So According to these tests results, we should change the part of the
>> qemu_ram_remap() function (in the 2nd patch) to something like:
>>
>> + if (ram_block_discard_range(block, offset +
block->fd_offset,
>> + length) != 0) {
>> + /*
>> + * Fold back to using mmap(), but it cannot
repair a
>> + * shared hugetlbfs region. In this case we fail.
>> + */
>
>
> But why do we special-case hugetlb here? How would mmap(MAP_FIXED) help
> to discard dirty pagecache data in a mmap(MAD_SHARED, fd) mapping?
You can see the behavior with the test case.
But for Qemu, we could decide to ignore that, and choose to fail in the
generic case:
+ /*
+ * Fold back to using mmap(), but it should not
repair a
+ * shared file memory region. In this case we fail.
+ */
+ if (block->fd >= 0 && qemu_ram_is_shared(block)) {
+ error_report("Shared memory poison recovery
failure addr: "
+ RAM_ADDR_FMT "@" RAM_ADDR_FMT "",
+ length, addr);
+ exit(1);
+ }
Do you think this would be more secure ?
HTH,
William.
---------------------------------
#include <sys/types.h>
#include <sys/mman.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <errno.h>
#include <stdint.h>
#include <signal.h>
#include <string.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <setjmp.h>
#define PAGEMAP_ENTRY 8
#define GET_BIT(X,Y) (X & ((uint64_t)1<<Y)) >> Y
#define GET_PFN(X) X & 0x7FFFFFFFFFFFFF
const int __endian_bit = 1;
#define is_bigendian() ( (*(char*)&__endian_bit) == 0 )
#define ALLOC_PAGES 2
#define myFile "FILE"
static sigjmp_buf jmpbuf;
/*
* Generate an error on the given page.
*/
static void memory_error_advise(void* virtual_page) {
int ret;
printf("Poisoning 4k at %p\n", virtual_page);
if (sigsetjmp(jmpbuf, 1) == 0) {
ret = madvise(virtual_page, 4096, MADV_HWPOISON);
if (ret)
printf("Poisoning failed - madvise: %s", strerror(errno));
}
}
static void print_physical_address(uint64_t virt_addr) {
char path_buf [0x100];
FILE * f;
uint64_t read_val, file_offset, pfn = 0;
long pgsz;
unsigned char c_buf[PAGEMAP_ENTRY];
pid_t my_pid = getpid();
int status, i;
sprintf(path_buf, "/proc/%u/pagemap", my_pid);
f = fopen(path_buf, "rb");
if(!f){
printf("Error! Cannot open %s\n", path_buf);
exit(EXIT_FAILURE);
}
pgsz = getpagesize();
file_offset = virt_addr / pgsz * PAGEMAP_ENTRY;
status = fseek(f, file_offset, SEEK_SET);
if(status){
perror("Failed to do fseek!");
fclose(f);
exit(EXIT_FAILURE);
}
for(i=0; i < PAGEMAP_ENTRY; i++){
int c = getc(f);
if(c==EOF){
fclose(f);
exit(EXIT_FAILURE);
}
if(is_bigendian())
c_buf[i] = c;
else
c_buf[PAGEMAP_ENTRY - i - 1] = c;
}
fclose(f);
read_val = 0;
for(i=0; i < PAGEMAP_ENTRY; i++){
read_val = (read_val << 8) + c_buf[i];
}
if(GET_BIT(read_val, 63)) { // Bit 63 page present
pfn = GET_PFN(read_val);
} else {
printf("Page not present !\n");
}
if(GET_BIT(read_val, 62)) // Bit 62 page swapped
printf("Page swapped\n");
if (pfn == 0) {
printf("Virt address translation 0x%llx failed\n");
exit(EXIT_FAILURE);
}
printf("Data pages at 0x%llx physically 0x%llx\n",
(unsigned long long)virt_addr, (unsigned long long)pfn * pgsz);
}
/*
* SIGBUS handler to display the given information.
*/
static void sigbus_action(int signum, siginfo_t *siginfo, void *ctx) {
printf("Signal %d received\n", signum);
printf("\tcode %d\t\tSignal code\n", siginfo->si_code);
printf("\taddr 0x%llx\tMemory location\n", siginfo->si_addr);
printf("\tsi_addr_lsb %d\n", siginfo->si_addr_lsb);
if (siginfo->si_code == 4) { /* BUS_MCEERR_AR */
fprintf(stderr, "siglongjmp used\n");
siglongjmp(jmpbuf, 1);
}
}
static void read_write(void* addr, int nb_pages, char* prefix) {
int i;
fprintf(stderr, "Reading and writing the first %d pages content:\n",
nb_pages);
if (sigsetjmp(jmpbuf, 1) == 0) {
// read the strings at the beginning of each page.
for (i=0; i < nb_pages; i++) {
printf("Read: %s", ((char *)addr+ i*4096));
}
// also write something
for (i=0; i < 2; i++) {
sprintf(((char *)addr + i*4096), "%s %d\n", prefix, i);
printf("Wrote: %s %d\n", prefix, i);
}
}
}
int main(int argc, char ** argv) {
int opt, fd, i;
struct sigaction my_sigaction;
uint64_t virt_addr, phys_addr;
void *local_pnt, *v;
struct stat statbuf;
off_t s;
// Need to have the CAP_SYS_ADMIN capability to get PFNs values in
pagemap.
if (getuid() != 0) {
fprintf(stderr, "Usage: %s needs to run as root\n", argv[0]);
exit(EXIT_FAILURE);
}
// attach our SIGBUS handler.
my_sigaction.sa_sigaction = sigbus_action;
my_sigaction.sa_flags = SA_SIGINFO | SA_NODEFER | SA_SIGINFO;
if (sigaction(SIGBUS, &my_sigaction, NULL) == -1) {
perror("Signal handler attach failed");
exit(EXIT_FAILURE);
}
fd = open(myFile, O_RDWR);
if (fd == -1) {
perror("open");
exit(EXIT_FAILURE);
}
if (fstat(fd, &statbuf) == -1) {
perror("fstat");
exit(EXIT_FAILURE);
}
s = statbuf.st_size;
if (s < 2*4096) {
fprintf(stderr, "File must be at least 2 pages large\n");
exit(EXIT_FAILURE);
}
printf("Mapping %d bytes from file %s\n", s, myFile);
local_pnt = mmap(NULL, s, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0);
if (local_pnt == MAP_FAILED) {
perror("mmap");
exit(EXIT_FAILURE);
}
read_write(local_pnt, 2, "Initial mem page");
virt_addr = (uint64_t)local_pnt;
print_physical_address(virt_addr);
print_physical_address(virt_addr+getpagesize());
// Explicit error
memory_error_advise((void*)virt_addr);
// Remap the poisoned page
fprintf(stderr, "Remapping the poisoned page\n");
v = mmap(local_pnt, 4092, PROT_READ|PROT_WRITE,
MAP_SHARED|MAP_FIXED, fd, 0);
if ((v == MAP_FAILED) || (v != local_pnt)) {
perror("mmap");
}
read_write(local_pnt, 2, "Rewrite mem page");
print_physical_address(virt_addr);
print_physical_address(virt_addr+getpagesize());
return 0;
}
