Issue in copy_string_from_task function

[sahaamit]

PeachOS64BitCourse/PeachOS64Bit/src/task/task.c

Lines 208 to 213 in 87c13b2

	paging_map(task_desc, phys_tmp, phys_tmp, PAGING_IS_WRITEABLE | PAGING_IS_PRESENT | PAGING_ACCESS_FROM_ALL);
	// Switch to the user pages
	task_page_task(task);
	// we are now on the page tables of the task
	strncpy(tmp, virtual, max);

Here, it is mapping virtual address phys_tmp in task's page table, not tmp. Hence, virtual address phys_tmp is only accessible once it switches to page table of user task. Ideally, it should copy from user text from virtual to phys_tmp instead of tmp. I think It won't cause any issue if kzalloc() returns linear address for tmp variable where virtual and physical address is same. So line 213 should be strncpy(phys_tmp, virtual, max); to handle the case.

[nibblebits]

Hello,

I disagree with your suggested change, while we are on ring zero and the kernel is mapped into the user process memory their should be some seperation when working between them, if we implemented your version we assume that the kernel will always be mapped into user space memory in the future which might not be the case.

While your correct its a simplier idea I would avoid it thats my opinion on the matter, but we can keep the issue on the Github repository incase students would prefer your way

Thanks
Daniel McCarthy

[nibblebits]

Wait a moment actually no your solution is solid and accurate, I correct myself here this is a better solution. It will be fixed in a new lesson, but in this repository: https://github.com/nibblebits/PeachOS64BitModuleTwo

Thanks