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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel




[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux