EtchedPixels / FUZIX

FuzixOS: Because Small Is Beautiful
Other
2.13k stars 267 forks source link

Fixing library p_tab to match kernel p_tab #1070

Closed veremenko-y closed 1 month ago

veremenko-y commented 1 month ago

Fixes #1069

Okay, this turned out nastier than I thought. It is caused specifically by the ps.c:475

ppid_slot[i] = ptab[i].p_tab.p_pptr - ptab[0].p_tab.p_pptr;

So as it turns out it was broken before and after struct p_tab change, it just was breaking differently. Library/struct p_tab actually has different size than Kernel/struct p_tab. Because Kernel/p_tab is larger, subtraction does not result in a value divisible by Library/p_tab, which I believe is UB.

before psize change
# ps -la                                                                               
ppid_slot[0] = 0. Address difference 0x00000000. Misallignment: 0 Ptab size: 0x0058    
ppid_slot[1] = 0. Address difference 0x00000000. Misallignment: 0 Ptab size: 0x0058    
ppid_slot[2] = 1. Address difference 0x0000005c. Misallignment: 4 Ptab size: 0x0058    

after psize change
# ps -la   
ppid_slot[0] = 0. Address difference 0x00000000. Misallignment: 0 Ptab size: 0x005c    
ppid_slot[1] = 0. Address difference 0x00000000. Misallignment: 0 Ptab size: 0x005c    
ppid_slot[2] = 14248. Address difference 0x00000060. Misallignment: 4 Ptab size: 0x005c

What actually breaks is line 341 where we try to get the process in the slot 14248...

printf("%5d ", ptab[ppid_slot[i]].p_tab.p_pid);
veremenko-y commented 1 month ago

This is the test code:

diff --git a/Applications/util/ps.c b/Applications/util/ps.c
index 6e50cafc9..48cb93256 100644
--- a/Applications/util/ps.c
+++ b/Applications/util/ps.c
@@ -473,6 +473,12 @@ int do_ps(void)
            return 1;
        }
        ppid_slot[i] = ptab[i].p_tab.p_pptr - ptab[0].p_tab.p_pptr;
+       printf("ppid_slot[%d] = %d. Address difference 0x%08x. Misallignment: %d Ptab size: 0x%04x\n",
+           i,
+           ppid_slot[i],
+           (void*)ptab[i].p_tab.p_pptr - (void*)ptab[0].p_tab.p_pptr,
+           ((void*)ptab[i].p_tab.p_pptr - (void*)ptab[0].p_tab.p_pptr) % sizeof(ptab[i].p_tab),
+           sizeof(ptab[i].p_tab));
        /* Learn our tty internal reference as we go */
        if (ptab[i].p_tab.p_status && ptab[i].p_tab.p_pid == pid)
            tty = ptab[i].p_tab.p_tty;
EtchedPixels commented 1 month ago

Ick.. I will take a look at fixing that somewhat more robustly Thanks for chasing it down

veremenko-y commented 1 month ago

Ick.. I will take a look at fixing that somewhat more robustly Thanks for chasing it down

I figured you'd want to look at it yourself. Do you want me to close the PR or keep it open?