Bug 11408 - (patch queued) another possible buffer-overflow bug in applicom.c
Summary: (patch queued) another possible buffer-overflow bug in applicom.c
Status: CLOSED CODE_FIX
Alias: None
Product: Drivers
Classification: Unclassified
Component: Flash/Memory Technology Devices (show other bugs)
Hardware: All Linux
: P1 normal
Assignee: Alan
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-08-22 15:07 UTC by Zvonimir Rakamaric
Modified: 2008-12-09 22:04 UTC (History)
2 users (show)

See Also:
Kernel Version: 2.6.26
Subsystem:
Regression: ---
Bisected commit-id:


Attachments

Description Zvonimir Rakamaric 2008-08-22 15:07:22 UTC
Hi,

first of all, I appreciate your quick response regarding the bug related to the same file (applicom.c) I submitted recently (Bug #11397). If you ever get to taking a closer look and maybe fixing that one, here is another one from the same file.

Problem Description:

Here is the part of code from applicom.c (function ac_ioctl) showing what I think is a possible buffer-overflow bug:

.....
  IndexCard = adgl->num_card-1;
	 
  if(cmd != 0 && cmd != 6 &&
      ((IndexCard >= MAX_BOARD) || !apbs[IndexCard].RamIO)) {
    static int warncount = 10;
    if (warncount) {
      printk( KERN_WARNING "APPLICOM driver IOCTL, bad board number %d\n",(int)IndexCard+1);
      warncount--;
    }
    kfree(adgl);
    return -EINVAL;
  }

  switch (cmd) {
		
  case 0:
    pmem = apbs[IndexCard].RamIO;
....


Explanation:

If cmd == 0 (or cmd == 6) then IndexCard is not going to be checked for size in this if statement:
  if(cmd != 0 && cmd != 6 &&
      ((IndexCard >= MAX_BOARD) || !apbs[IndexCard].RamIO)) {

which means that when we hit this case:
  case 0:
    pmem = apbs[IndexCard].RamIO;

it is possible that IndexCard >= MAX_BOARD and we have a buffer overflow.

There is a possibility, however, that there is an implicit, complex invariant that whenever cmd == 0 or cmd == 6, IndexCard is never going to be out of bounds and therefore doesn't have to be checked. Then this bug wouldn't happen, but I think this is still a pretty bas coding practice.

Any explanation would be greatly appreciated.

Thanks!
-- Zvonimir
Comment 1 Alan 2008-09-23 03:21:55 UTC
Grabbing to fix
Comment 2 Alan 2008-10-02 06:22:27 UTC
Patch Queued
Comment 3 Alan 2008-10-16 06:24:46 UTC
Merged into git tree for 2.6.28

Note You need to log in before you can comment on or make changes to this bug.