From: Ludovic Rousseau (ludovic.rousseaugmail.com)
Date: Mon Feb 01 2010 - 08:52:07 CST

• Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

2010/2/1 Max Vozeler <max.vozelerlsexperts.de>:
> Hi Ludovic,
>
> On Mon, Feb 01, 2010 at 02:06:58PM +0100, Ludovic Rousseau wrote:
>> 2010/2/1 Max Vozeler <max.vozelerlsexperts.de>:
>> > This patch adds a workaround for unusual behaviour of Broadcom
>> > BCM5880/5881 readers. They misreport an absent card as "Hardware
>> > Error" in GetSlotStatus.
>> >
>> > To work around it, we turn the HW_ERROR answer to GetSlotStatus
>> > into "No error, no card inserted".
>> >
>> > Since we need some way to look up the required quirk based on
>> > USB vendor/product ID, this patch adds some simple quirk lookup
>> > functions which can also be used for future quirks.
>>
>> I don't like the quirks lookup functions idea.
>
> Fair enough. Like I said, I can see how you wouldn't want to
> have two quirk mechanisms in upstream libccid.
>
>> In the code I detect bogus readers using:
>> if (SPR532 == ccid_descriptor->readerID)
>
> See adapted patch below.
>
>        Max
>
> --- ccid-1.3.11/src/ccid.h
> +++ ccid-1.3.11/src/ccid.h
> -172,6 +172,8 typedef struct
>  #define OZ776          0x0B977762
>  #define OZ776_7772     0x0B977772
>  #define SPR532         0x04E6E003
> +#define BCM5880                0x0a5c5800
> +#define BCM5881                0x0a5c5801
>  #define MYSMARTPAD     0x09BE0002
>  #define CHERRYXX44     0x046a0010
>  #define CL1356D                0x0B810200
> --- ccid-1.3.11/src/commands.c
> +++ ccid-1.3.11/src/commands.c
> -1009,8 +1009,35 again_status:
>  #endif
>                ccid_error(buffer[ERROR_OFFSET], __FILE__, __LINE__, __FUNCTION__);    /* bError */
>
> +               /*
> +                * Special case for BCM5880/5881 readers since they
> +                * answer with HW_ERROR if no card is inserted. We still
> +                * log the error but turn it into "no card inserted".
> +                */
> +
> +               if (BCM5880 == ccid_descriptor->readerID
> +                       || BCM5881 == ccid_descriptor->readerID)
> +               {
> +                       log_msg(PCSC_LOG_ERROR, "%s:%d:%s %s",
> +                               __FILE__, __LINE__, __FUNCTION__,
> +                               "GetSlotStatus workaround for BCM5880/1");
> +
> +                       /*
> +                        * bmICCStatus: 2 ("No ICC present")
> +                        * bmCommandStatus: 0 ("Processed without error")
> +                        */
> +
> +                       buffer[STATUS_OFFSET] = 0x02;
> +
> +                       /*
> +                        * Clear error register.
> +                        */
> +
> +                       buffer[ERROR_OFFSET] = 0x00;
> +               }
> +
>                /* card absent or mute is not an communication error */
> -               if (buffer[ERROR_OFFSET] != 0xFE)
> +               else if (buffer[ERROR_OFFSET] != 0xFE)
>                        return_value = IFD_COMMUNICATION_ERROR;
>        }

Souldn't you test the buffer[ERROR_OFFSET] value to be sure you do not
ignore valid error codes?

In the case of BCM588. I suggest to just correct the
buffer[ERROR_OFFSET] and buffer[STATUS_OFFSET] bytes.
And wrap the code inside a #ifdef BOGUS_BROADCOM_FIRMWARE like for the
other BOGUS_* patches.

--
 Dr. Ludovic Rousseau

_______________________________________________
Muscle mailing list
Musclelists.musclecard.com
http://lists.drizzle.com/mailman/listinfo/muscle

• Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]