On 20-05-2014 22:52:41, Dan Carpenter wrote:
> 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.
I fixed this (not sent yet).
>
> > + 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().
What do you mean by "preserves the error code from
BcmFlash2xCorruptSig()" ? I check the return value from the function
I introduced and if it is not STATUS_SUCCESS, I return it. I cannot
see the problem?
Is this the bug you mentioned above?
Thank you for your comments.
--
Mit freundlichen Grüßen,
Kind regards,
Matthias Beyer
Proudly sent with mutt.
Happily signed with gnupg.
Attachment:
pgpwPibdFK2Fi.pgp
Description: PGP signature
_______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
