Skip to content

Keep PCSC reader flags when disconnected #2600

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
Jakuje merged 1 commit into from
Sep 30, 2022
Merged

Keep PCSC reader flags when disconnected #2600

Jakuje merged 1 commit into from
Sep 30, 2022

Conversation

@xhanulik
Copy link
Member

@xhanulik xhanulik commented Sep 2, 2022

As discussed in #2415, when C_GetSlotList() is called before C_WaitForSlotEvent(), removing reader is not properly detected. The C_GetSlotList() calls sc_ctx_detect_readers(), which sets reader->flags to SC_READER_REMOVED and therefore card_detect_all() frees some reader resources and sets slot->reader to NULL. However, card_removed() also overwrites reader->flags from SC_READER_REMOVED to 0.

OpenSC/src/pkcs11/slot.c

Lines 398 to 411 in ca01aa7

if (reader->flags & SC_READER_REMOVED) {
card_removed(reader);
/* do not remove slots related to this reader which would be
* possible according to PKCS#11 2.20 and later, because NSS can't
* handle a shrinking slot list
* https://bugzilla.mozilla.org/show_bug.cgi?id=1613632 */
/* Instead, remove the relation between reader and slot */
for (j = 0; j<list_size(&virtual_slots); j++) {
sc_pkcs11_slot_t *slot = (sc_pkcs11_slot_t *) list_get_at(&virtual_slots, j);
if (slot->reader == reader) {
slot->reader = NULL;
}
}

Then, C_WaitForSlotEvent() calls card_detect_all() again, corresponding slots have slot->reader = NULL and therefore they are reclaimed via create_slot(). The removed reader is set back to the slot, but the event flags slot->events are overwritten with 0, and the event can not be detected from slot->events (the event flags would be normally changed in card_detect()).

OpenSC/src/pkcs11/slot.c

Lines 145 to 161 in ca01aa7

} else {
DEBUG_VSS(slot, "Reusing this old slot");
/* reuse the old list of logins/objects since they should be empty */
list_t logins = slot->logins;
list_t objects = slot->objects;
memset(slot, 0, sizeof *slot);
slot->logins = logins;
slot->objects = objects;
}
slot->login_user = -1;
slot->id = (CK_SLOT_ID) list_locate(&virtual_slots, slot);
init_slot_info(&slot->slot_info, reader);
slot->reader = reader;

This behaviour can be fixed by keeping reader flags after disconnecting in reader-pcsc.c, then card_detect_all() does not reclaim the slot with the removed reader and event flags are kept for C_WaitForSlotEvent().

Fixes #2415.

Checklist
  • PKCS#11 module is tested
  • Windows minidriver is tested
  • macOS tokend is tested

@xhanulik
Keep pcsc reader flags when disconnected
62cf2c4 
When the reader is removed, the reader->flags are set to
SC_READER_REMOVED. card_detect_all() then frees resources and
sets slot->reader to NULL. During this, sc_card_disconnect()
is called, setting reader->flags to 0. In a subsequent call
to card_detect_all() with the reader still removed, the corresponding
slot is reclaimed, and slot->reader set back to the removed reader,
but slot->events is overwritten, which causes the C_WaitForSlot()
not to recognize event on the slot.
@Jakuje Jakuje mentioned this pull request Sep 12, 2022
Closed
@dengert dengert mentioned this pull request Sep 12, 2022
Closed
@Jakuje Jakuje merged commit a39d70b into OpenSC:master Sep 30, 2022
@Jakuje
Copy link
Member

Jakuje commented Sep 30, 2022

Thank you!

@xhanulik xhanulik deleted the reader branch October 10, 2022 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@frankmorgner frankmorgner frankmorgner approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

C_WaitForSlotEvent(CKF_DONT_BLOCK) no longer works properly

3 participants

@xhanulik @Jakuje @frankmorgner