> Subject: puzzle for puzzle lovers
> Date: Tuesday 04 October 2011
> From: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> To: kernel-janitors@xxxxxxxxxxxxxxx
>
> Here is something that might amuse someone.
>
> Smatch reports a read past the end of the array in rocket.c
> drivers/tty/rocket.c +2168 init_ISA(77)
> error: buffer overflow calling init_r_port. param 0. 7 >= 4
>
> drivers/tty/rocket.c
> 657 init_completion(&info->close_wait);
> 658 info->flags &= ~ROCKET_MODE_MASK;
> 659 switch (pc104[board][line]) {
> 660 case 422:
> 661 info->flags |= ROCKET_MODE_RS422;
> 662 break;
>
> pc104[] is a 4 element array.
>
> board comes from for loop in rp_init().
>
> 2315 for (i = 0; i < NUM_BOARDS; i++) {
> 2316 if (init_ISA(i))
> 2317 isa_boards_found++;
> 2318 }
>
> NUM_BOARDS is is 8, so according to Smatch "board" can be 7
At that point in the code, 'board' can only be 0-3. The init_ISA(i)
function will return immediately if isa_io_addr[i] is 0. isa_io_addr[]
is a static 8-element array and only the first 4 elements are ever
assigned to. Therefore, isa_io_addr[4] is guaranteed to be 0, and the
loop above is never called with 'board' greater than 3.
> and no one knows what line is.
'line' is the serial port index within that board (e.g. 0-7 on a
8-port board, 0-31 on a 32-port board).
> Weird huh?
pc104 is a 4-element array of pointers. Those pointers point to
8-element arrays. 'board' can be 0-3, and 'line' can be 0-7.
The problem is that there are 32-port ISA boards, so I think pc104_1
through pc104_4 should be 32-element arrays. I suspect this hasn't
caused any failures because the flags set based on pc104[board][line]
are only used in the case of PC104 boards, and PC104 boards never have
more than 8 ports.
IOW, for boards with more than 8 ports, there is a read-past-end-of-array
but the result isn't used. Hmm -- there may be one case where the
bogus result _is_ used but it would take some work to figure out if
that happens or not. It would be better to just change pc104_1
through pc104_4 from 8-element arrays to 32-element arrays.
> But the code is ancient from before the git era so no one knows what
> it's supposed to do.
I can explain "it" to you if you specify to what "it" refers.
> Unless you are clever enough to solve this mystery.
If it would make people happy, feel free to make pc104[] a
MAX_NUMBEROF_BOARDS sized array. It's not possible to configure more
than 4 ISA boards, but if the objective is to make a static code
analyzer happy, then go ahead.
> This code gets run during init so presumably it got tested often
> ten years ago and it works.
Diclaimer: Comtrol offers no support for the in-tree driver. The
driver we officially support is at ftp://ftp.comtrol.com/html/default.htm
--
Grant Edwards
grant.edwards@xxxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
