On Thu, Aug 9, 2018 at 5:35 AM Christophe Fergeau <cfergeau@xxxxxxxxxx> wrote: > > On Thu, Aug 09, 2018 at 09:26:35AM +0200, Jakub Jelen wrote: > > On Wed, 2018-08-08 at 14:08 -0400, Jason Andryuk wrote: > > > On Wed, Aug 8, 2018 at 11:33 AM Jakub Jelen <jjelen@xxxxxxxxxx> > > > wrote: > > > > > > > > On Wed, 2018-08-08 at 16:51 +0200, Marc-André Lureau wrote: > > > > > Hi > > > > > > > > > > On Tue, Jul 24, 2018 at 8:34 PM, Jason Andryuk < > > > > > jandryuk@xxxxxxxxx> > > > > > wrote: > > > > > > If a command fails, card_status will not match > > > > > > VCARD_DONE. That > > > > > > will > > > > > > trigger the assert and abort the process. Instead, handle > > > > > > VCARD_FAIL and > > > > > > return an error in that case. Client software can then deal > > > > > > with > > > > > > the > > > > > > error, and we continue running to handle future commands. > > > > > > > > > > > > This can be triggered by removing the physical smartcard mid- > > > > > > operation. > > > > > > > > > > There are other paths, like invalid instruction on > > > > > cac_common_process_apdu_read() > > > > > > > > The invalid instructions should return valid response with error > > > > indicated in SW (status words). The referenced function has the > > > > default > > > > VCARD_FAIL value is in the code somehow bogus in case we would like > > > > to > > > > fail early or fail to handle some case (?). > > > > > > > > The VCARD_FAIL option is really about more serious issues as Jason > > > > is > > > > pointing out. > > > > > > > > Handling the error here, rather than segfaulting in assert later > > > > sounds > > > > like a good idea. But from reading the code, I still can not find a > > > > path where we could encounter this value here. > > > > > > > > From what I see, all the paths here return either VCARD_DONE. Can > > > > you > > > > advice during which operation did you encounter this error? > > > > > > My setup is qemu <-> vscclient <-> pcscd with passthru: > > > vscclient -e 'use_hw=yes hw_type=passthru' > > > > > > In a Windows VM, I ran `certutil -scinfo` from a cmd window. While > > > it > > > was running, I pulled out my smart card. Without my patch, vscclient > > > terminates. With it, vscclient continues running. > > > > > > The call stack is: > > > vreader_xfr_bytes > > > vcard_process_apdu > > > vcard_process_applet_apdu > > > apdu_cb > > > > > > apdu_cb can return VCARD_FAIL for send_receive or > > > vcard_response_new_data failure. > > > > > > So you think src/capcsc.c:apdu_cb() should return VCARD_DONE but use > > > vcard_response_set_status_bytes to write an appropriate status word? > > > > I was referring mostly to the card emulation code which I know better > > and which is safe in this way it returns the correct status words. > > > > I don't know much about the passthrough mode. But you are right -- it > > can return VCARD_FAIL in case of the transmit to the real card fails > > and I believe failing this way is a correct thing to do. > > > > Acked-by: Jakub Jelen <jjelen@xxxxxxxxxx> > > Thanks for the patch and review, I've added the reproducer description > to the commit log and pushed this. Thank you. -Jason _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel