lkml.org 
[lkml]   [2003]   [May]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
Subject[CHECKER] aacraid user pointer use
From
Stanford checker said:
---------------------------------------------------------
[BUG] at least bad programming practice. file_opetations.ioctl ->
aac_cfg_ioctl -> aac_do_ioctl -> close_getadapter_fib ->
aac_close_fib_context. All other functions called by aac_do_ioctl assume
arg is a user pointer. It is unknown what will happen.

/home/junfeng/linux-2.5.63/drivers/scsi/aacraid/
commctrl.c:277:aac_close_fib_context:
ERROR:TAINTED:277:277: dereferencing tainted ptr 'fibctx' [Callstack:
/home/junfeng/linux-2.5.63/drivers/scsi/sg.c:1002:aac_cfg_ioctl((tainted
3)) ->
/home/junfeng/linux-2.5.63/drivers/scsi/aacraid/
linit.c:671:aac_do_ioctl((tainted
2)) ->
/home/junfeng/linux-2.5.63/drivers/scsi/aacraid/
commctrl.c:421:close_getadapter_fib((tainted
1)) ->
/home/junfeng/linux-2.5.63/drivers/scsi/aacraid/
commctrl.c:348:aac_close_fib_context((tainted
1))]

while (!list_empty(&fibctx->fibs)) {
struct list_head * entry;
/*
* Pull the next fib from the fibs
*/

Error --->
entry = fibctx->fibs.next;
list_del(entry);
fib = list_entry(entry, struct hw_fib, header.FibLinks);
fibctx->count--;
---------------------------------------------------------

As it turns out, the driver is fine. It is dereferencing a
user-supplied pointer (fibctx), but it keeps a list of valid structures
and has already made sure fibctx is one of them before using it. This
is in contrast to the PCMCIA code, which uses a magic number to verify
(as discussed yesterday) rather than keeping a list of all valid
pointers.

The attached aacraid patch may help the checker, and should be
functionally equivalent... but isn't necessary.

--
Hollis Blanchard
IBM Linux Technology Center
===== drivers/scsi/aacraid/commctrl.c 1.2 vs edited =====
--- 1.2/drivers/scsi/aacraid/commctrl.c Wed May 7 21:08:36 2003
+++ edited/drivers/scsi/aacraid/commctrl.c Thu May 29 17:33:17 2003
@@ -214,8 +214,8 @@
if (found == 0)
return -EINVAL;

- if((fibctx->type != FSAFS_NTC_GET_ADAPTER_FIB_CONTEXT) ||
- (fibctx->size != sizeof(struct aac_fib_context)))
+ if((aifcp->type != FSAFS_NTC_GET_ADAPTER_FIB_CONTEXT) ||
+ (aifcp->size != sizeof(struct aac_fib_context)))
return -EINVAL;
status = 0;
spin_lock_irqsave(&dev->fib_lock, flags);
@@ -224,16 +224,16 @@
* -EAGAIN
*/
return_fib:
- if (!aac_list_empty(&fibctx->hw_fib_list)) {
+ if (!aac_list_empty(&aifcp->hw_fib_list)) {
struct aac_list_head * entry;
/*
* Pull the next fib from the fibs
*/
- entry = (struct aac_list_head*)(ulong)fibctx->hw_fib_list.next;
+ entry = (struct aac_list_head*)(ulong)aifcp->hw_fib_list.next;
aac_list_del(entry);

hw_fib = aac_list_entry(entry, struct hw_fib, header.FibLinks);
- fibctx->count--;
+ aifcp->count--;
spin_unlock_irqrestore(&dev->fib_lock, flags);
if (copy_to_user(f.fib, hw_fib, sizeof(struct hw_fib))) {
kfree(hw_fib);
@@ -244,11 +244,11 @@
*/
kfree(hw_fib);
status = 0;
- fibctx->jiffies = jiffies/HZ;
+ aifcp->jiffies = jiffies/HZ;
} else {
spin_unlock_irqrestore(&dev->fib_lock, flags);
if (f.wait) {
- if(down_interruptible(&fibctx->wait_sem) < 0) {
+ if(down_interruptible(&aifcp->wait_sem) < 0) {
status = -EINTR;
} else {
/* Lock again and retry */
@@ -341,11 +341,11 @@
if(found == 0)
return 0; /* Already gone */

- if((fibctx->type != FSAFS_NTC_GET_ADAPTER_FIB_CONTEXT) ||
- (fibctx->size != sizeof(struct aac_fib_context)))
+ if((aifcp->type != FSAFS_NTC_GET_ADAPTER_FIB_CONTEXT) ||
+ (aifcp->size != sizeof(struct aac_fib_context)))
return -EINVAL;
spin_lock_irqsave(&dev->fib_lock, flags);
- status = aac_close_fib_context(dev, fibctx);
+ status = aac_close_fib_context(dev, aifcp);
spin_unlock_irqrestore(&dev->fib_lock, flags);
return status;
}
\
 
 \ /
  Last update: 2005-03-22 13:35    [W:0.029 / U:0.396 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site