lkml.org 
[lkml]   [2006]   [Jun]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [Patch] Off by one in drivers/usb/serial/usb-serial.c
    * Mikael Pettersson (mikpe@it.uu.se) wrote:
    > On Wed, 21 Jun 2006 23:28:17 +0200, Eric Sesterhenn wrote:
    > > this fixes coverity id #554. since serial table
    > > is defines as serial_table[SERIAL_TTY_MINORS] we
    > > should make sure we dont acess with an index
    > > of SERIAL_TTY_MINORS.
    > >
    > > Signed-off-by: Eric Sesterhenn <snakebyte@gmx.de>
    > >
    > > --- linux-2.6.17-git2/drivers/usb/serial/usb-serial.c.orig 2006-06-21 23:24:07.000000000 +0200
    > > +++ linux-2.6.17-git2/drivers/usb/serial/usb-serial.c 2006-06-21 23:25:12.000000000 +0200
    > > @@ -83,7 +83,7 @@ static struct usb_serial *get_free_seria
    > >
    > > good_spot = 1;
    > > for (j = 1; j <= num_ports-1; ++j)
    > > - if ((i+j >= SERIAL_TTY_MINORS) || (serial_table[i+j])) {
    > > + if ((i+j >= SERIAL_TTY_MINORS-1)||(serial_table[i+j])) {
    > > good_spot = 0;
    > > i += j;
    > > break;
    >
    > Where is the access coverity complained about? If it's the serial_table[i+j]
    > quoted above, then the original code is OK since i+j < SERIAL_TTY_MINORS is
    > an invariant in that subexpression.
    >
    > And the other accesses to serial_table[] in get_free_serial() are also only
    > done when the index is < SERIAL_TTY_MINORS.

    guess i was too quick on that one, sorry. Here is the coverity
    report for completeness.

    Event assignment: Assigning "1" to "j"
    Also see events: [overrun-local]
    At conditional (11): "j <= (num_ports - 1)" taking true path
    At conditional (16): "j <= (num_ports - 1)" taking true path

    85 for (j = 1; j <= num_ports-1; ++j)

    Event overrun-local: Overrun of static array "serial_table" of size 255
    at position 255 with index variable "(i + j)"
    Also see events: [assignment]
    At conditional (12): "(i + j) >= 255" taking true path
    At conditional (17): "(i + j) >= 255" taking false path

    86 if ((i+j >= SERIAL_TTY_MINORS) ||
    (serial_table[i+j])) {
    87 good_spot = 0;
    88 i += j;
    89 break;
    90 }




    greetings, Eric

    -
    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: 2006-06-26 01:01    [W:0.090 / U:151.080 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site