Bugzilla will be upgraded to version 5.0 on a still to be determined date in the near future. The original upgrade date has been delayed.
First Last Prev Next    This bug is not in your last search results.
Bug 13745 - megaraid 1e08 driver rejected
megaraid 1e08 driver rejected
Status: CLOSED RAWHIDE
Product: Red Hat Linux
Classification: Retired
Component: kernel (Show other bugs)
7.1
i386 Linux
high Severity high
: ---
: ---
Assigned To: Michael K. Johnson
Winston rc1
:
Depends On:
Blocks:
  Show dependency tree / graph
 
Reported: 2000-07-11 15:52 EDT by Matt Domsch
Modified: 2008-05-01 11:37 EDT (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2000-08-09 13:03:17 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---

Attachments (Terms of Use)
megaraid.patch (5.12 KB, patch)
2000-08-04 21:16 EDT, Matt Domsch 		no flags Details | Diff
megaraid-1.09.tgz (24.45 KB, application/octet-stream)
2000-08-07 23:06 EDT, Matt Domsch 		no flags Details
Add an attachment (proposed patch, testcase, etc.)

  None (edit)
Description Matt Domsch 2000-07-11 15:52:01 EDT 
megaraid driver 1b08b and 1e08 (functionally identical) both have this 
same problem which caused Alan to reject the driver update.  AMI needs 
Alan's help to resolve this issue properly.


Rejected. This is completely unsafe

#if LINUX_VERSION_CODE > 0x020024
/*
 * usage of the function copy from user is used in case of data more than
 * 4KB.  This is used only with adapters which supports more than 8 logical
 * drives.  This feature is disabled on kernels earlier or same as 2.0.36  
 * as the uaccess.h file is not available with those kernels.
 */
   
  if (SCpnt->cmnd[0] == IOCTL_CMD_NEW) {
            /* use external data area for large xfers  */
     /* If cmnd[0] is set to IOCTL_CMD_NEW then *
      *   cmnd[4..7] = external user buffer     *
      *   cmnd[8..11] = length of buffer        *
      *                                         */
      u32 xfer_size = *((u32 *)&SCpnt->cmnd[8]);  
      user_area = *((char **)&SCpnt->cmnd[4]);    
      if (verify_area(VERIFY_READ, user_area, xfer_size)) {
          printk("megaraid: Got bad user address.\n");
          SCpnt->result = (DID_ERROR << 16);
          callDone (SCpnt);
          return NULL;
	}
      mega_build_user_sg(user_area, xfer_size, pScb, mbox);

  }
#endif

You cannot go around peeking into unlocked user pages. Nor can you assume
they are even reachable by PCI. You can scatter gather fine but use
get_free_page() and copy the data.


Alan
Comment 1 Glen Foster 2000-07-30 18:55:18 EDT 
This defect is considered MUST-FIX for Winston Release-Candidate #1

... this bug needs some serious attention (based on Alan's comments), Doug. 
Please do what you can to take care of this defect ASAP.  I don't want to have
another "post-release AMI Mega-RAID" issue like we did in Zoot. :-)
Comment 2 Alan Cox 2000-07-30 20:56:15 EDT 
I have talked with Peter about doing it the way it was done before and I think
we are both happy
on that and the nasty hack removed. Check with PeterJ
Comment 3 Matt Domsch 2000-08-04 21:04:21 EDT 
Doug's second megaraid patch dereferences a NULL pointer somewhere when the 
driver loads.  He thought it might not be 100% ready, but more a proof-of-
concept.  I'm afraid I haven't had time to debug it properly, so if Michael or 
PeterJ can, that's great.
Comment 4 Matt Domsch 2000-08-04 21:16:17 EDT 
Created attachment 2075 [details]
megaraid.patch
Comment 5 Matt Domsch 2000-08-04 21:17:02 EDT 
Doug's comments on making the above patch:

OK, attached is a new version of the patch I sent last night.  It fixes the
fact that my first patch was really begging me not to leak 32K of kmalloc'ed
RAM with every ioctl (doh!!) and it also moves to per Scb semaphores so that
multiple ioctl commands won't step on each other.  Finally, I'm not a
semaphore expert, so should I be adding a wait queue to the semaphore init, or
does the kernel use a generic one in this case?  I think it just uses a
generic one and the right spot will get woken up when the up() happens, but I
could be wrong.  Anyway, the process I have in there should be sound.  The
main thing is that you can't hold the io_request_lock or the driver lock or
any other lock for that matter during either the down() or the copy_to_user(),
the rest of the time you should be locked.
Comment 6 Michael K. Johnson 2000-08-07 15:25:37 EDT 
Giving this one back to you, Doug!  Thanks...
Comment 7 Matt Domsch 2000-08-07 15:33:36 EDT 
AMI found the NULL pointer dereference bug that Doug had left in there.  The
rest of it looks great!  AMI and Dell are testing, and will resubmit ASAP.

Peter, when you're confident, can you release a 1.09 driver?
-Matt
Comment 8 Michael K. Johnson 2000-08-07 22:44:31 EDT 
ASAP is putting it very lightly.  Peter, if you are
not confident enough to release a 1.09 driver, could
you please at least send the fix for the NULL pointer
deref for us to drop in for initial tests -- I need
it in a few *hours* to make RC1.
Comment 9 Matt Domsch 2000-08-07 23:06:21 EDT 
Created attachment 2234 [details]
megaraid-1.09.tgz
Comment 10 Matt Domsch 2000-08-07 23:08:35 EDT 
The above megaraid-1.09.tgz is exactly the 1b0b driver, + Doug's patch, + 
Manoj's fix to Doug's patch.  I'm sending this now because time is short.  I'm 
not precluding PeterJ from sending a "better" version.  We've tested this 
driver somewhat, and it certainly solves the NULL pointer dereference issue.
Comment 11 Michael K. Johnson 2000-08-08 00:05:29 EDT 
Thanks for your quick response, Matt.  I'm in the
process of integrating the patch now.  I expect
to give you a kernel tomorrow for testing, and it
will have this version of the driver in it.
Comment 12 Michael K. Johnson 2000-08-08 09:37:28 EDT 
OK, Matt, there is now a 2.2.16-19.1 kernel in your
private directory on gribble.  Please test.  Peter,
if you want a copy, I can give one to you, or you
can get it from Matt, whichever works best.
Comment 13 Michael K. Johnson 2000-08-09 12:56:26 EDT 
Since the driver is integrated, I'm closing this.
Matt, I'm a bit confused about the megamgr program
bug that you reported earlier; want to open a new
bug report about that bug if it is a real bug?
Comment 14 Matt Domsch 2000-08-09 13:03:15 EDT 
I don't know if it's a real bug yet or not, but I'll open a new issue if/when I 
can prove either way.
Note You need to log in before you can comment on or make changes to this bug.
First Last Prev Next    This bug is not in your last search results.