Bug#46709: Mach lets processes write to I/O ports

[Marcus Brinkmann]

On Thu, May 03, 2001 at 11:53:41AM +0300, Kalle Olavi Niemitalo wrote:
> Letting processes write to EGA ports isn't that awful (with
> today's sync-protected monitors), but if Mach also lets them
> write to ports used by IDE or SCSI, then it "introduces a
> security hole on systems where you install the package."

I have found the bugs that allow users to write to I/O ports without
requesting permission.  In i386/i386/ktss.c, only the last byte of the
io map of the KERNEL_TSS is set to 0xff (access forbidden), all other
are left as they are (all zeros? random? I don't know, but suspect all
zero).  Threads that don't have their own TSS use the KERNEL_TSS, and
this means all threads by default (see pcb.c).  BTW, I am pretty sure there
is an off by one error, which means that the code writes into the byte past
the bitmap, rather than the last byte of it (I verified that the bitmap
indeed starts at ktss, and that means that ktss + 0x10000/8 is off by one).

This is the first bug.  The second bug is that the i386_io_port_add function
that creates the user tss initializes it with access_all being true, which
means that it is initialized to all zero.  So a thread requesting any access
will always get all (I suspect).  The two places marked with /* XXX */ in
the iopb.c file seem to indicate that this is first a correct interpretation
of the code and that the author knew that this had to be fixed.  Replacing the
TRUE at the XXX with FALSE, and setting all bytes in the map to 0xff in the
KERNEL_TSS should fix this, but I did not test that part.

The reason I could not continue testing the user tss is that
the i386_io_port_add call can't find the device -> io_port mapping.  I tried
both the kd and iopl device, and neither worked.  This requires more
debugging (maybe tomorrow).  We decided by now that this is the wrong approach
anyway, and that all this code is doomed to be replaced by a somewhat
different approach.  However, I think it is not a bad idea to fix this,
especially as it doesn't seem to hard, before going on and messing with it
(well, at least for me as I have to learn more details about the tss stuff
first before I can have a go at the new interface).

I am including my preliminary patch and my two test programs (incomplete,
just for reference) that show the i386_io_port_add problem.  The test
program you submitted fails with Illegal Instruction if you apply my patch.

BTW, I noticed that there are reminiscences of io map support in 
thread_setstatus and thread_getstatus, with a reference to a i386_io_port_map
call that seems to be intended to set/get a complete bitmask.  This cruft can
be cleaned up when we have a go at the new interface.

Thanks,
Marcus

--- /mnt/marcus/gnu/cvs/gnumach/i386/i386/ktss.c        Tue Feb 25 22:27:10 1997
+++ i386/i386/ktss.c    Sun Oct  7 05:15:18 2001
@@ -39,6 +39,7 @@
 void
 ktss_init()
 {
+       int i;
        /* XXX temporary exception stack */
        static exception_stack[1024];
 
@@ -52,8 +53,9 @@
        ktss.esp0 = (unsigned)(exception_stack+1024);
        ktss.io_bit_map_offset = sizeof(ktss);
 
-       /* Set the last byte in the I/O bitmap to all 1's.  */
-       ((unsigned char*)&ktss)[sizeof(ktss)+65536/8] = 0xff;
+       /* Set all bytes in the I/O bitmap to all 1's.  */
+       for (i = sizeof(ktss); i < sizeof(ktss)+65536/8; i++)
+         ((unsigned char*)&ktss)[i] = 0xff;
 
        /* Load the TSS.  */
        ltr(KERNEL_TSS);
--- /mnt/marcus/gnu/cvs/gnumach/i386/i386/iopb.c        Tue Feb 25 22:27:09 1997
+++ i386/i386/iopb.c    Sun Oct  7 05:00:09 2001
@@ -270,7 +270,7 @@
        register iopb_tss_t ts;
 
        ts = (iopb_tss_t) kalloc(sizeof (struct iopb_tss));
-       io_tss_init(ts, TRUE);          /* XXX */
+       io_tss_init(ts, FALSE);
        return ts;
 }
 
@@ -357,7 +360,7 @@
                simple_unlock(&iopb_lock);
 
                new_io_tss = (iopb_tss_t) kalloc(sizeof(struct iopb_tss));
-               io_tss_init(new_io_tss, TRUE);  /* XXX */
+               io_tss_init(new_io_tss, FALSE);
 
                goto Retry;
            }

-- 
`Rhubarb is no Egyptian god.' Debian http://www.debian.org brinkmd@debian.org
Marcus Brinkmann              GNU    http://www.gnu.org    marcus@gnu.org
Marcus.Brinkmann@ruhr-uni-bochum.de
http://www.marcus-brinkmann.de

ioport2.c
Description: Text Data

ioport3.c
Description: Text Data