lkml.org 
[lkml]   [2004]   [Jan]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH][RFC] variable size and signedness issues in ldt.c - potential problem?

    On Thu, 8 Jan 2004, Jesper Juhl wrote:

    > I'm hunting the kernel source for any potential problem I can find (and
    > hopefully fix), and I've come across something that looks like a
    > possible problem in arch/i386/kernel/ldt.c
    >
    > First thing that looks suspicious is this bit in read_ldt() :
    >
    > for (i = 0; i < size; i += PAGE_SIZE) {
    > ...
    > }
    >
    > 'i' is a plain int while 'size' is an 'unsigned long' leaving the
    > possibility that if size contains a value greater than what a signed int
    > can hold then this code won't do the right thing, either 'i' will wrap
    > around to zero and the loop will never exit or something "unknown" will
    > happen (as far as I know, what happens when an int overflows is
    > implementation defined). [...]

    but the user does not control 'newsize'. Can you outline a scenario in
    where the value could overflow?

    > The second thing is that in the body of the 'for' loop there is this
    > comparison :
    >
    > if (bytes > PAGE_SIZE)

    no, the value of bytes is really limited. Again, can you suggest a
    scenario in where this could overflow?

    > I know that the only user of read_ldt() and write_ldt() is
    > sys_modify_ldt() , and the arguments for read_ldt and write_ldt thus
    > have to match sys_modify_ldt, but why is the 'bytecount' argument for
    > sys_modify_ldt an 'unsigned long' and the return type an 'int' ? The
    > signedness of the return type makes sense given that it't supposed to
    > return -1 on error. But on success, in the case where it calls read_ldt,
    > it's supposed to return the actual number of bytes read. But if the
    > number of bytes to read is given as an unsigned long, and the number
    > actually read exceeds the size of a signed int then the return value
    > will get truncated upon return - how can that be right? [...]

    LDT size is limited by LDT_ENTRY_SIZE*LDT_ENTRIES. We explicitly truncate
    bytecount to this range so unsigned vs. signed makes no difference.

    > [...] And if the return value can never exceed what a signed int can
    > hold, then why is it possible to request an unsigned long amount of
    > bytes to read in the first place?

    that's quite common for the interface definitions. Since we are on x86
    unsigned long == unsigned int.

    > and finally a purely style related thing (sure, call me pedantic); in both
    > read_ldt() and write_ldt() 'mm' is declared as
    >
    > struct mm_struct * mm = current->mm;

    yep, you are right, this is the wrong style.

    Ingo
    -
    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/

    \
     
     \ /
      Last update: 2005-03-22 13:59    [W:0.025 / U:88.392 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site