[PATCH v2 3/3] Staging: bcm: Outsourced IsFlash2x() handling

[Dan Carpenter]

No problem, let me explain the bug better.

On Wed, May 21, 2014 at 06:14:53PM +0200, Matthias Beyer wrote:
> > > +	ULONG ulDSDMagicNumInUsrBuff = 0;
> > > +
> > > +	Status = BcmFlash2xCorruptSig(Adapter, Adapter->eActiveDSD);
> > > +	if (Status != STATUS_SUCCESS) {
> > > +		if (((stNVMReadWrite->uiOffset + stNVMReadWrite->uiNumBytes) !=
> > > +			Adapter->uiNVMDSDSize) ||
> > > +			(stNVMReadWrite->uiNumBytes < SIGNATURE_SIZE)) {
> > > +
> > > +			BCM_DEBUG_PRINT(Adapter, DBG_TYPE_OTHERS, OSAL_DBG,
> > > +					DBG_LVL_ALL,
> > > +					"DSD Sig is present neither in Flash nor User provided Input..");
> > > +			up(&Adapter->NVMRdmWrmLock);
> > > +			kfree(pReadData);
> > > +			return Status;
> > > +		}
> > > +
> > > +		ulDSDMagicNumInUsrBuff = ntohl(*(PUINT)(pReadData +
> > > +					stNVMReadWrite->uiNumBytes -
> > > +					SIGNATURE_SIZE));
> > > +		if (ulDSDMagicNumInUsrBuff != DSD_IMAGE_MAGIC_NUMBER) {
> > > +			BCM_DEBUG_PRINT(Adapter, DBG_TYPE_OTHERS, OSAL_DBG,
> > > +					DBG_LVL_ALL,
> > > +					"DSD Sig is present neither in Flash nor User provided Input..");
> > > +			up(&Adapter->NVMRdmWrmLock);
> > > +			kfree(pReadData);
> > > +			return Status;
> > > +		}

		<--- We should be returning STATUS_SUCCESS here inside
		     the if statement.
> > > +	}
        <---  And we should be returning STATUS_SUCCESS outside the if
              statement.

> > > +
> > > +	return Status;

The way that it is written in this patch we return STATUS_SUCCESS
outside the if statement, but inside the if statement we return the
failure code from BcmFlash2xCorruptSig().

In other words you could write it like this:

	ULONG ulDSDMagicNumInUsrBuff = 0;

	Status = BcmFlash2xCorruptSig(Adapter, Adapter->eActiveDSD);
	if (Status == STATUS_SUCCESS)
		return STATUS_SUCCESS;  <---- Your version returns success here

	if (((stNVMReadWrite->uiOffset stNVMReadWrite->uiNumBytes) !=
		Adapter->uiNVMDSDSize) ||
		(stNVMReadWrite->uiNumBytes < SIGNATURE_SIZE)) {
			BCM_DEBUG_PRINT(Adapter, DBG_TYPE_OTHERS, OSAL_DBG,
				DBG_LVL_ALL,
				"DSD Sig is present neither in Flash nor User provided Input..");
		up(&Adapter->NVMRdmWrmLock);
		kfree(pReadData);
		return Status;
	}

	ulDSDMagicNumInUsrBuff = ntohl(*(PUINT)(pReadData +
				stNVMReadWrite->uiNumBytes -
				SIGNATURE_SIZE));
	if (ulDSDMagicNumInUsrBuff != DSD_IMAGE_MAGIC_NUMBER) {
		BCM_DEBUG_PRINT(Adapter, DBG_TYPE_OTHERS, OSAL_DBG,
				DBG_LVL_ALL,
				"DSD Sig is present neither in Flash nor User provided Input..");
		up(&Adapter->NVMRdmWrmLock);
		kfree(pReadData);
		return Status;
	}

	return STATUS_SUCCESS;  <---- You need to return success here as well.

regards,
dan carpenter