Skip to content

Memory mapped files not written to disk correctly #18

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
rick-masters opened this issue Jan 12, 2023 · 1 comment · Fixed by #19
Closed

Memory mapped files not written to disk correctly #18

rick-masters opened this issue Jan 12, 2023 · 1 comment · Fixed by #19
Labels
bug Something isn't working

Comments

@rick-masters
Copy link
Contributor

The contents of memory mapped files created using mmap are not written correctly.

The following test program (adapted from https://stackoverflow.com/questions/26259421/use-mmap-in-c-to-write-into-memory) can reproduce the problem:

#include <stdlib.h>
#include <unistd.h>
#include <stdio.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <sys/mman.h> /* mmap() is defined in this header */
#include <fcntl.h>
#include <string.h>

void err_quit(char *msg)
{
    puts(msg);
    exit(1);
}
int main (int argc, char *argv[])
{
 int fdin, fdout;
 char *src, *dst;
 struct stat statbuf;
 int mode = 0x0777;

 if (argc != 3)
   err_quit ("usage: a.out <fromfile> <tofile>");

 /* open the input file */
 if ((fdin = open (argv[1], O_RDONLY)) < 0)
   {printf("can't open %s for reading", argv[1]);
    return 0;
   }

 /* open/create the output file */
 if ((fdout = open (argv[2], O_RDWR | O_CREAT | O_TRUNC, mode )) < 0)//edited here
   {printf ("can't create %s for writing", argv[2]);
    return 0;
   }

 /* find size of input file */
 if (fstat (fdin,&statbuf) < 0)
   {printf ("fstat error");
    return 0;
   }

 /* go to the location corresponding to the last byte */
 if (lseek (fdout, statbuf.st_size - 1, SEEK_SET) == -1)
   {printf ("lseek error");
    return 0;
   }

 /* write a dummy byte at the last location */
 if (write (fdout, "", 1) != 1)
   {printf ("write error");
     return 0;
   }

 /* mmap the input file */
 if ((src = mmap (0, statbuf.st_size, PROT_READ, MAP_SHARED, fdin, 0))
   == (caddr_t) -1)
   {printf ("mmap error for input");
    return 0;
   }

 /* mmap the output file */
 if ((dst = mmap (0, statbuf.st_size, PROT_READ | PROT_WRITE,
   MAP_SHARED, fdout, 0)) == (caddr_t) -1)
   {printf ("mmap error for output");
    return 0;
   }

 /* this copies the input file to the output file */
 memcpy (dst, src, statbuf.st_size);
 return 0;

} /* main */

The following commands will run the test.

cc mmaptest.c -o mmaptest
dd if=/dev/zero of=testin bs=1 count=10037
./mmaptest testin testout
diff testin testout

The result should be that testin and testout are the same.
Instead testout is mostly filled with "random" data.

The root cause of the problem is this line of code:

Fiwix/mm/mmap.c

Line 288 in abc14a0

write_page(pg, vma->inode, addr, length);

The code above writes the full length of the file from each and every page. So, even though a page is at most 4096 bytes, the write_page routine is trying to copy data from each page far past the end of the page boundary. This puts random data into the file.

A PR with a suggested fix is forthcoming.

@mikaku
Copy link
Owner

mikaku commented Jan 13, 2023

You found another bug!
I've completed a new massive package build with your patch and I haven't experienced any regression, good.
Thank you very much!

@mikaku mikaku added the bug Something isn't working label Jan 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants