Found a bug in JamesM multitasking code, in clone table

[JSmith2]

Hello, I recently found a bug in his code and would like to share it. Some parts of my OS are based of of his code and the time has come to improve the multitasking. After several days of banging my head against the wall, the bug was really really simple. The problem was that when I wanted to clone a page directory, the first time would work, and then the second time, it would assert (or triple fault, I added an assert in find_smallest_hole, which checks the magic of the found hole). The problem was in the clone table function which is embedded within the clone page directory function. Here is the bug...

before:

Code: Select all

  // Make a new page table, which is page aligned.
  page_table_t *table = (page_table_t*)kmalloc_ap(sizeof(page_table_t), physAddr);
  // Ensure that the new table is blank.
  memset((u8int*)table, 0, sizeof(page_directory_t));

I did not notice that table was allocated a page_table_t (4096) and memset with the size of a page_directory_t (8192)
after changing the memset's clear size to sizeof(page_table_t):

Code: Select all

  // Make a new page table, which is page aligned.
  page_table_t *table = (page_table_t*)kmalloc_ap(sizeof(page_table_t), physAddr);
  // Ensure that the new table is blank.
  memset((u8int*)table, 0, sizeof(page_table_t));

Everything worked perfectly!!!

I hope this is useful to anyone else who is also trying to debug this problem.

[sortie]

Note that the JamesM tutorial contains errors on purpose - it's intended to break so the programmer is forced to actually learn what is going on and fix that. It also advocates doing very nasty things involving stacks when creating kernel threads (by forking them! wtf) and other nasty tricks. Keep an eye on what you are doing and don't do it if it seems bad.

[JSmith2]

Well, I will never be sure if that bug was on purpose or not, but my intentions in posting this topic are to only make someone else's life easier and to possibly officially fix this problem.

[dozniak]

Theoretically, size of the page table and page directory on x86 should be the same

[JSmith2]

Well, the problem was due to them being different, the sizeof(page_directory_t) was twice as large as sizeof(page_table_t), when I would clear the table of any junk, it would clear past where the allocator told it to end.

[piranha]

Well, the problem was due to them being different, the sizeof(page_directory_t) was twice as large as sizeof(page_table_t), when I would clear the table of any junk, it would clear past where the allocator told it to end.

Well, then the problem lies in the size of page_directory_t, because on x86, they are the same size (4KB).

-JL

[JSmith2]

Yes, yes, I understand that, it is just that I have to look in and see why they are different. Also, even if those two typedefs are the same in size, it still is more correct to allocate sizeof(page_table_t) and memset with sizeof(page_table_t), not sizeof(page_directory_t), regardless of them having equivalent sizes or not.