2.4.23: user/kernel pointer bugs in drivers/pcmcia/ds.c

From: Robert T. Johnson (rtjohnso_at_eecs.berkeley.edu)
Date: 01/21/04

  • Next message: Randy.Dunlap: "Re: List 'linux-dvb' closed to public posts"
    To: dhinds@zen.stanford.edu
    Date:	21 Jan 2004 13:25:43 -0800
    
    

    I think there are several user/kernel pointer bugs in
    drivers/pcmcia/ds.c:ds_ioctl(). I haven't made a patch because the
    fixes aren't obvious (to me). Here they are:

    ** ds.c:748
        if (cmd & IOC_IN) copy_from_user((char *)&buf, (char *)arg, size);
        // The fields of buf are now under user control

    ** ds.c:809
        pcmcia_get_first_window(&buf.win_info.handle, &buf.win_info.window);

    ** cs.c:1199
    int pcmcia_get_first_window(window_handle_t *win, win_req_t *req)
    {
        if ((win == NULL) || ((*win)->magic != WINDOW_MAGIC))
            return CS_BAD_HANDLE;
        return pcmcia_get_window(win, 0, req);
    }

    So *win is a pointer under user control, and ((*win)->magic derefs it.
    I think it gets derefed again in pcmcia_get_window. The call to
    pcmcia_get_next_window() on ds.c:812 has the the same problem.

    Similarly:

    ** ds.c:805
            ret = pcmcia_get_next_region(s->handle, &buf.region);

    ** bulkmem.c: 439
    int pcmcia_get_next_region(client_handle_t handle, region_info_t *rgn)
    {
        if (CHECK_HANDLE(handle))
            return CS_BAD_HANDLE;
        return match_region(handle, rgn->next, rgn);
    } /* get_next_region */

    The memory pointed to by rgn is under user control, so rgn->next
    is under user control.

    ** bulkmem.c: 406
    static int match_region(client_handle_t handle, memory_handle_t list,
                            region_info_t *match)
    {
        while (list != NULL) {
            if (!(handle->Attributes & INFO_MTD_CLIENT) ||
                (strcmp(handle->dev_info, list->dev_info) == 0)) {
                *match = list->info;
                return CS_SUCCESS;
            }
            list = list->info.next;
        }
        return CS_NO_MORE_ITEMS;
    } /* match_region */

    So list is a pointer under user control, and strcmp(.., list->dev_info)
    derefs it
    (and list->info, etc.).

    There are similar problems with the calls
    pcmcia_get_first_region() on ds.c:802,
    pcmcia_get_mem_page() on ds.c:815 (first argument)

    Thanks for looking at these.

    Best,
    Rob

    P.S. These bugs were found using the source code verification
    tool, CQual, developed by Jeff Foster, myself, and others, and available
    from http://www.cs.umd.edu/~jfoster/cqual/.

    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/


  • Next message: Randy.Dunlap: "Re: List 'linux-dvb' closed to public posts"

    Relevant Pages

    • Re: [patch] Real-Time Preemption, -VP-2.6.9-rc4-mm1-U2
      ... mm/shmem.c:1362: warning: assignment makes pointer from integer without a cast ... send the line "unsubscribe linux-kernel" in ...
      (Linux-Kernel)
    • Re: setproctitle
      ... > know, by default the title is the args, but the set operation would ... > pointer. ... When you have patches for such proposed changes I'll review them then. ... send the line "unsubscribe linux-kernel" in ...
      (Linux-Kernel)
    • Re: how to detect a 32 bit process on 64 bit kernel
      ... >>from user space by implementing a write file operation. ... >>The structure includes a pointer and so the format varies ... send the line "unsubscribe linux-kernel" in ...
      (Linux-Kernel)
    • Re: [patch 1/16] s390: klist bus_find_device & driver_find_device callback.
      ... > criterium and to return a pointer to it with which the caller can ... do something later on with this pointer, ... stack and doing my logic in the callback function itself, ... send the line "unsubscribe linux-kernel" in ...
      (Linux-Kernel)
    • Re: [PATCH] Driver Core update for 2.6.4
      ... get/put logic with an atomic counter and that "bloat of a pointer" just ... got lost in the noise of the extra kernel code size increase:) ... send the line "unsubscribe linux-kernel" in ...
      (Linux-Kernel)