FDOS/kernel

SYS kernel config incorrectly swaps wrong size 19 to 14, should be to 5 or 6

ecm-pushbx opened this issue · 5 comments

The exeflat utility wrote a wrong kernel CONFIG block length of 19 when UPX compression was introduced. In this git repo the problem was corrected on 2012-10-15 in a260927#diff-2477748a9e5dedc77cffa1fc943860aa18c9fe23567aa2550c1f5c7e60bb8053L266

The same commit introduced a wrong fix to the problem:

  /* check if config settings old UPX header and adjust */
  if (cfg->ConfigSize == 19)
    cfg->ConfigSize = 14;  /* ignore 'nused87654321' */

The wrong header was introduced in https://hg.pushbx.org/ecm/fdkernel.svn/rev/af1dfee339ef#l65.109 on 2002-01-23, original SVN revision at https://sourceforge.net/p/freedos/svn/343/

In the SVN repo the header was fixed in https://hg.pushbx.org/ecm/fdkernel.svn/rev/977d55fa398a on 2016-02-06, original SVN revision at https://sourceforge.net/p/freedos/svn/1735/

I don't know the reasoning behind the initial error. However, all the wrong errors want to say 6, not 19 nor 14. 14 would be correct if you counted the CONFIG signature (6) + length field (2) + actual length of fields (6). The CONFIG block included in kernel.asm always got it right, and consistently does not count the signature nor the length field itself:

dw configend-configstart; size of config area

We should also note in the users and providers of the structure that we should not provide a legitimate length of 19 as it could be misdetected as the wrong header. Or perhaps detect the "nused" padding bytes, but then we would have to make sure not to ever write these byte values legitimately.

The initial introduction of the wrong length field actually had "unused" behind the used configuration fields however, quoth https://hg.pushbx.org/ecm/fdkernel.svn/rev/af1dfee339ef#l65.109

    /* UPX HEADER jump $+2+size */
    static char JumpBehindCode[] = {
      /* kernel config header - 32 bytes */
      0xeb, 0x1b,               /*     jmp short realentry */
      'C', 'O', 'N', 'F', 'I', 'G', 32 - 2 - 6 - 2 - 3, 0,      /* WORD */
      0,                        /* DLASortByDriveNo            db 0  */
      1,                        /* InitDiskShowDriveAssignment db 1  */
      2,                        /* SkipConfigSeconds           db 2  */
      0,                        /* ForceLBA                    db 0  */
      1,                        /* GlobalEnableLBAsupport      db 1  */
      'u', 'n', 'u', 's', 'e', 'd',     /* unused filler bytes                              */
      8, 7, 6, 5, 4, 3, 2, 1,
      /* real-entry: jump over the 'real' image do the trailer */
      0xe9, 0, 0                /* 100: jmp 103 */
    };

Apparently I am also unable to count: The original error wants us to treat the length field as 5, not 6. That is why it reads "unused", not just the truncated "nused".

Additional problem: If SYS CONFIG writes changes to the CONFIG block it will also rewrite the length field. So both involved wrong lengths, 19 and 14, should be treated as 5 or 6.

Fixed with commit 569337a