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

Dan Carpenter dan.carpenter at oracle.com
Tue May 20 19:52:41 UTC 2014


On Tue, May 20, 2014 at 05:12:46PM +0200, Matthias Beyer wrote:
> This patch outsources the code from the IsFlash2x() check in
> bcm_char_ioctl_nvm_rw() function to shorten it.
> 

This patch introduces a bug.  Please fix and resend.

Also move the function forward so we don't need a declaration.

> +
> +static int handle_flash2x_adapter(struct bcm_mini_adapter *Adapter,
> +	PUCHAR pReadData, struct bcm_nvm_readwrite *stNVMReadWrite)
> +{
> +	/*
> +	 * New Requirement:-
> +	 * DSD section updation will be allowed in two case:-
> +	 * 1.  if DSD sig is present in DSD header means dongle
> +	 * is ok and updation is fruitfull
> +	 * 2.  if point 1 failes then user buff should have
> +	 * DSD sig. this point ensures that if dongle is
> +	 * corrupted then user space program first modify
> +	 * the DSD header with valid DSD sig so that this
> +	 * as well as further write may be worthwhile.
> +	 *
> +	 * This restriction has been put assuming that
> +	 * if DSD sig is corrupted, DSD data won't be
> +	 * considered valid.
> +	 */
> +	INT Status = STATUS_FAILURE;

Don't initialize Status here.  It's misleading because we overwrite it
immediately.

> +	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;
> +		}
> +	}
> +
> +	return Status;

This should be return "STATUS_SUCCESS".  The comment explains that if
we are able to write a corrupt signature the that is success.  Or
alternatively if we are able to get the DSD signature from the user
then that is also success.

The original code worked as described in the comment but your version
preserves the error code from BcmFlash2xCorruptSig().

regards,
dan carpenter



More information about the devel mailing list