[Openib-windows] [PATCH] MTHCA: Secure FW update

Fab Tillier ftillier at silverstorm.com
Fri Mar 31 13:25:47 PST 2006


Hi Leo,

This patch performs some cleanup of the FW update paths.  The changes are:

- The fw_access_ctrl function needs to return ib_api_status_t, not NTSTATUS.
This is a bug that was carried over from the MT23108 driver (I'll send a patch
for that soon).

- There is no need to track the process ID in the CR space mappings - there is
no way for the process ID to change for a particular h_um_ca handle, as it is
tied to an IBAL instance which is tied to a single process.

- MmMapLockedPagesSpecifyCache for UserMode throws and exception if it fails,
rather than returning NULL like for kernel calls.  This needs to be trapped with
__try/__except.

- Added some checks on buffers - it is possible for buf_len to be non-zero while
p_buf is NULL.

- Fab

Index: hw/mthca/kernel/hca_driver.c
===================================================================
--- hw/mthca/kernel/hca_driver.c	(revision 266)
+++ hw/mthca/kernel/hca_driver.c	(working copy)
@@ -345,6 +345,9 @@
 
 	PAGED_CODE();
 
+	if( !p_buffer )
+		return STATUS_INVALID_PARAMETER;
+
 	if (p_BusInterface)
 	{
 
@@ -439,7 +442,6 @@
 	struct list_head list;
 	PMDL	p_mdl;
 	PVOID	va;
-	PEPROCESS p_pcs;
 } mthca_map_space;
 
 static NTSTATUS
@@ -461,7 +463,7 @@
 	HCA_ENTER( HCA_DBG_PNP );
 
 	// sanity checks
-	if ( buf_size < sizeof *p_res ) {
+	if ( buf_size < sizeof *p_res || !p_buf ) {
 		status = STATUS_INVALID_PARAMETER;
 		goto err_invalid_params;
 	}
@@ -485,7 +487,7 @@
 		if ( ka == NULL) {
 			HCA_PRINT(TRACE_LEVEL_ERROR  , HCA_DBG_SHIM,
 				("No kernel mapping of CR space.\n") );
-			status = STATUS_UNSUCCESSFUL;
+			status = STATUS_INSUFFICIENT_RESOURCES;
 			goto err_map_to_kernel;
 		}
 		p_ext->bar[HCA_BAR_TYPE_HCR].virt = ka;
@@ -505,12 +507,16 @@
 	MmBuildMdlForNonPagedPool(p_mdl);
 	
 	// map the buffer into user space 
-	ua = MmMapLockedPagesSpecifyCache( p_mdl, UserMode, MmNonCached, 
-		NULL, FALSE, NormalPagePriority );
-	if (ua == NULL) {
-		HCA_PRINT(TRACE_LEVEL_ERROR  , HCA_DBG_SHIM, 
+	__try
+	{
+		ua = MmMapLockedPagesSpecifyCache( p_mdl, UserMode, MmNonCached,
+			NULL, FALSE, NormalPagePriority );
+	}
+	__except(EXCEPTION_EXECUTE_HANDLER)
+	{
+		HCA_PRINT(TRACE_LEVEL_ERROR , HCA_DBG_SHIM,
 			("MmMapLockedPagesSpecifyCache failed.\n") );
-		status =  STATUS_UNSUCCESSFUL;
+		status = STATUS_INSUFFICIENT_RESOURCES;
 		goto err_map_to_user;
 	}
 	
@@ -521,7 +527,6 @@
 	// resource tracking
 	p_map->p_mdl = p_mdl;
 	p_map->va = ua;
-	p_map->p_pcs = PsGetCurrentProcess();
 	list_add_tail(&p_map->list, &p_context->map_list);
 	
 	up( &p_context->mutex );
@@ -553,12 +558,11 @@
 	unmap_crspace *parm = (unmap_crspace *)p_buf;
 	mthca_map_space *p_map, *p_tmp;
 	int found = FALSE;
-	PEPROCESS  p_pcs = PsGetCurrentProcess();
 
 	HCA_ENTER( HCA_DBG_PNP );
 
 	// sanity checks
-	if ( buf_size < sizeof *parm ) {
+	if ( buf_size < sizeof *parm || !p_buf ) {
 		status = STATUS_INVALID_PARAMETER;
 		goto err_invalid_params;
 	}
@@ -569,7 +573,7 @@
 	// look for the mapping info
 	list_for_each_entry_safe(p_map, p_tmp, &p_context->map_list, list, 
 		mthca_map_space, mthca_map_space) {
-		if (p_map->va == parm->va && p_map->p_pcs == p_pcs) {
+		if (p_map->va == parm->va) {
 			found = TRUE;
 			break;
 		}
@@ -579,7 +583,7 @@
 		HCA_PRINT(TRACE_LEVEL_ERROR  , HCA_DBG_SHIM, 
 			("Not found internal info for unmappipng.%p for PID
%d.\n" , 
 			parm->va, (int)(INT_PTR)PsGetCurrentProcessId()));
-		status =  STATUS_UNSUCCESSFUL;
+		status = STATUS_INVALID_PARAMETER;
 		goto err_not_found;
 	}
 
@@ -638,7 +642,6 @@
 	PVOID						p_data;
 	ULONG						offset;
 	ULONG POINTER_ALIGNMENT		length;
-	ib_ci_op_t					*p_ci;
 	mlnx_hob_t					*p_hob;
 	struct ib_ucontext *			p_context;
 
@@ -646,28 +649,23 @@
 	UNREFERENCED_PARAMETER(num_handles);
 	UNREFERENCED_PARAMETER(p_umv_buf);
 
-	status =  STATUS_SUCCESS;
+	status = STATUS_INVALID_DEVICE_REQUEST;
 
-	if( p_umv_buf ) {
-		p_context = (struct ib_ucontext *)h_ca;
-		p_hob = HOB_FROM_IBDEV( ((struct ib_ucontext*)h_ca)->device );
-	}
-	else {
-		p_context = NULL;
-		p_hob = (mlnx_hob_t *)(const void *)h_ca;
-	}
+	if( !p_umv_buf )
+		return IB_UNSUPPORTED;
 
+	p_context = (struct ib_ucontext *)h_ca;
+	p_hob = HOB_FROM_IBDEV( ((struct ib_ucontext*)h_ca)->device );
 	p_dev_obj = (DEVICE_OBJECT *)EXT_FROM_HOB(p_hob)->cl_ext.p_self_do;
-	p_ci =  p_ci_op;
 
-	if ( !p_ci ||  !p_ci->buf_size )
-		return STATUS_INVALID_DEVICE_REQUEST;
+	if ( !p_ci_op || !p_ci_op->buf_size )
+		return IB_INVALID_PARAMETER;
 
-	length = p_ci->buf_size;
-	offset = p_ci->buf_info;
-	p_data = p_ci->p_buf;
+	length = p_ci_op->buf_size;
+	offset = p_ci_op->buf_info;
+	p_data = p_ci_op->p_buf;
 
-	switch ( p_ci->command )
+	switch ( p_ci_op->command )
 	{
 	case FW_MAP_CRSPACE:
 		status = __map_crspace(p_context, p_hob, p_data, length);
@@ -703,7 +701,7 @@
 			if_ready = 0;
 
BusInterface.InterfaceDereference((PVOID)BusInterface.Context);
 		}
-		return status;
+		return IB_SUCCESS;
 
 	case FW_OPEN_IF: // open BusInterface
 		if ( !if_ready )
@@ -713,13 +711,16 @@
 			if ( NT_SUCCESS( status ) )
 			{
 				if_ready = 1;
-				status = STATUS_SUCCESS;
 			}
 		}
-		return status;
+		else
+		{
+			status = STATUS_SUCCESS;
+		}
+		break;
 
 	default:
-		status = STATUS_NOT_SUPPORTED;
+		status = STATUS_INVALID_DEVICE_REQUEST;
 	}
 
 	if ( status != STATUS_SUCCESS )
@@ -730,9 +731,22 @@
 
BusInterface.InterfaceDereference((PVOID)BusInterface.Context);
 		}
 		HCA_PRINT( TRACE_LEVEL_ERROR, HCA_DBG_INIT, 
-			("fw_access_ctrl failed returns %08x.\n", status));
+			("fw_access_ctrl failed, ntstatus: %08x.\n", status));
 	}
-	return status;
+	switch( status )
+	{
+	case STATUS_SUCCESS:
+		return IB_SUCCESS;
+
+	case STATUS_INVALID_DEVICE_REQUEST:
+		return IB_UNSUPPORTED;
+
+	case STATUS_INSUFFICIENT_RESOURCES:
+		return IB_INSUFFICIENT_RESOURCES;
+
+	default:
+		return IB_ERROR;
+	}
 }
 
 static NTSTATUS
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mthca_fw_update.patch
Type: application/octet-stream
Size: 5121 bytes
Desc: not available
URL: <http://lists.openfabrics.org/pipermail/ofw/attachments/20060331/ec5bb562/attachment.obj>


More information about the ofw mailing list