Card driver should return SC_ERROR_INVALID_CARD on failures during initialisation

[dengert]

Issue: #943 The underlying issue was card/reader/pcsc was returning 90 00 when selecting AID which does not exist on the card. This appears to be some configuration issue and not a bug in the card driver. But the problem did uncover some other issues with how a failure in init is handled.

sc_connect_card() in card.c will call ops->match_card(card). If true is returned ops->init(card) will be called. If SC_ERROR_INVALID_CARD is returned, the loop will be continued. If not the loop will end.

One problem is card drivers expect if ops->init(card) is called then ops->finish(card) will be called to clean up. This is not done in the loop. This can cause memory leaks.

I would like to propose:

1. Evaluate the error codes in each driver to see if it should return SC_ERROR_INVALID_CARD indicating that the driver thinks that other drivers (if any) should be tried, or it is some other failure and no other drivers should be tried. (Maybe this should be in the loop?)

2. In the loop, if ops->init(card) returns SC_ERROR_INVALID_CARD, ops->finish(card) is called. and card->dvr_data = NULL;

3. Evaluate each driver to see if it depends on any fields in card that may have been changed by a previous card driver's init routine, and make sure these are reset if we continue the loop. We have done this already for many cases.

We have 37 card drivers (and iso7816.c)
ls card-*.c | wc

27 drivers define a *_finish routine:
grep "_finish(.*sc_card.*)" card-*.c
(card-openpgp.c is counted twice in the above grep).

28 drivers set the ops.finish:
grep "ops[.]finish.*=" card-*.c

10 drivers do not set ops.finish and use what iso7816.c which is a NULL pointer.
grep -L "ops[.]finish.*=" card-*.c

8 of these do not reference drv_data. The other 2 set drv_data = NULL.
grep -L "drv_data" card-*.c

3 cards may return SC_ERROR_INVALID_CARD to try and continue the loop:
grep SC_ERROR_INVALID_CARD card-*.c

Only card-openpgp.c in pgp_init does a cleanup by calling its own pgp_finish(card)

Also see #945 that only addressed one driver.

[frankmorgner]

I think it would be easiest if you propose a patch. I'm pretty sure that SC_ERROR_INVALID_CARD is what should be returned.

[dengert]

In sc_connect_card, the reusing of the card structure after a card driver returns SC_ERROR_INVALID_CARD assumes that the card driver has cleaned up anything it did in its *_init routine.

Above I said:

3 cards may return SC_ERROR_INVALID_CARD to try and continue the loop:
grep SC_ERROR_INVALID_CARD card-*.c

It is now 4 card drivers return SC_ERROR_INVALID_CARD:

• card-gids.c - Does not return it from gids_init. (so not a problem)
• card-dine.c - dnie_match matches an ATR and sets SC_CARD_TYPE_DNIE_TERMINATED. Then in dnie_init for that card type, it returns SC_ERROR_INVALID_CARD before doing anything else to card structure.
• card-mcrd.c - mcrd-init, if selecting the AID fails, SC_ERROR_INVALID_CARD is returned. But it leaves card->caps = SC_CARD_CAP_RNG;
• card-oberthur.c auth_init, if selecting the AID fails, SC_ERROR_INVALID_CARD is returned. But it leaves card->drv_data = data; and leaves card->caps = SC_CARD_CAP_RNG | SC_CARD_CAP_USE_FCI_AC;

All the drivers above do a match_atr for their own cards first. So the select AID is only done for those ATRs. The only issue could be if other drivers can also match on those ATRs.

card-dnie.c does not cause any problems, but could be simplified to not match that ATR or return no match for that ATR.

So one could call this a card-driver problem for now and ask @viktorTarasov to look at card-oberthur.c and @martinpaljak to look at card-mrcd.c

But new card drivers must make sure they clean up the card structure before returning SC_ERROR_INVALID_CARD from their *-init routines.

@frankmorgner You can leave this open as enhancement or call it a problem with 2 drivers or just close it.