Hi,

Please check the attached patch, it should be safe to apply. It mainly moves the pcsc dependency to the sub-plugin, so that the rdpdr_main can be smartcard independent. It compiles and runs ok without PCSC. This achieves the goal of making pcsc dependency in its own sub-plugin and can be packaged separately.

However, I think a bigger issue is in rdpdr_scard.c file. It's casting a pointer to uint32, which will almost certainly to crash on 64bit systems; and the pointer points to IRP struct, which is just a locally allocated struct and it's not safe to be accessed from another thread, otherwise it's also going to be crashed.

To solve the issue, I think we need to extend the device plugin API and provide a callback to the sub-plugin so that the sub-plugin can send messages in a safe way, then eventually we are able to remove rdpdr_scard.

Vic
>From 1102c547993ece4e66c6911a0c1f4dce7af44b13 Mon Sep 17 00:00:00 2001
From: Vic Lee <[email protected]>
Date: Wed, 4 May 2011 15:58:53 +0800
Subject: [PATCH] rdpdr: move PCSC dependency to smartcard sub-plugin and make
 rdpdr_main independent of compile option.


Signed-off-by: Vic Lee <[email protected]>
---
 channels/rdpdr/Makefile.am           |    7 +------
 channels/rdpdr/rdpdr_main.c          |    6 ------
 channels/rdpdr/smartcard/Makefile.am |    6 ++++--
 configure.ac                         |   14 +++++---------
 4 files changed, 10 insertions(+), 23 deletions(-)

diff --git a/channels/rdpdr/Makefile.am b/channels/rdpdr/Makefile.am
index d0fb71a..54db7e9 100644
--- a/channels/rdpdr/Makefile.am
+++ b/channels/rdpdr/Makefile.am
@@ -5,11 +5,6 @@ rdpdrdir = $(PLUGIN_PATH)
 
 rdpdr_LTLIBRARIES = rdpdr.la
 
-if WITH_SCARD
-SCARD_SOURCES = \
-	rdpdr_scard.h rdpdr_scard.c
-endif
-
 rdpdr_la_SOURCES = \
 	rdpdr_types.h \
 	rdpdr_main.c rdpdr_main.h \
@@ -18,7 +13,7 @@ rdpdr_la_SOURCES = \
 	devman.c devman.h \
 	irp.c irp.h \
 	irp_queue.c irp_queue.h \
-	$(SCARD_SOURCES)
+	rdpdr_scard.h rdpdr_scard.c
 
 rdpdr_la_CFLAGS = -I$(top_srcdir)/include -I$(srcdir)/../common \
 	-DPLUGIN_PATH=\"$(PLUGIN_PATH)\"
diff --git a/channels/rdpdr/rdpdr_main.c b/channels/rdpdr/rdpdr_main.c
index b40cdb1..38bf54f 100644
--- a/channels/rdpdr/rdpdr_main.c
+++ b/channels/rdpdr/rdpdr_main.c
@@ -33,9 +33,7 @@
 #include "irp_queue.h"
 #include "config.h"
 
-#ifdef WITH_SCARD
 #include "rdpdr_scard.h"
-#endif
 
 #define MAX(x,y)             (((x) > (y)) ? (x) : (y))
 
@@ -566,13 +564,11 @@ rdpdr_process_irp(rdpdrPlugin * plugin, char* data, int data_size)
 		LLOGLN(10, ("IRP enqueue event"));
 		irp_queue_push(plugin->queue, &irp);
 	}
-#ifdef WITH_SCARD
 	else if (irp.ioStatus == (RD_STATUS_PENDING | 0xC0000000)) /* smart card */
 	{
 		irp.ioStatus = RD_STATUS_PENDING; /* it was pushed into smart card queue: queueFirst, queueLast... */
 		LLOGLN(10, ("smart card irp must not be stored into plgugin->queue"));
 	}
-#endif
 	else if (irp.ioStatus != RD_STATUS_PENDING)
 	{
 		out = irp_output_device_io_completion(&irp, &out_size);
@@ -1046,9 +1042,7 @@ VirtualChannelEntry(PCHANNEL_ENTRY_POINTS pEntryPoints)
 	devman_load_device_service(plugin->devman, "printer");
 	devman_load_device_service(plugin->devman, "serial");
 	devman_load_device_service(plugin->devman, "parallel");
-#ifdef WITH_SCARD
 	devman_load_device_service(plugin->devman, "scard");
-#endif
 
 	plugin->ep.pVirtualChannelInit(&plugin->chan_plugin.init_handle, &plugin->channel_def, 1,
 		VIRTUAL_CHANNEL_VERSION_WIN2000, InitEvent);
diff --git a/channels/rdpdr/smartcard/Makefile.am b/channels/rdpdr/smartcard/Makefile.am
index ff2a678..8c08f5d 100644
--- a/channels/rdpdr/smartcard/Makefile.am
+++ b/channels/rdpdr/smartcard/Makefile.am
@@ -10,12 +10,14 @@ scard_la_SOURCES = \
 	scard_queue.c scard_queue.h \
 	scard_operations.c
 
-scard_la_CFLAGS = -I$(top_srcdir)/include -I$(srcdir)/../../common \
+scard_la_CFLAGS = @PCSCLITE_CFLAGS@ \
+	-I$(top_srcdir)/include -I$(srcdir)/../../common \
 	-I$(srcdir)/.. -DPLUGIN_PATH=\"$(PLUGIN_PATH)\"
 
 scard_la_LDFLAGS = -avoid-version -module
 
-scard_la_LIBADD = ../../common/libcommon.la
+scard_la_LIBADD = @PCSCLITE_LIBS@ \
+	../../common/libcommon.la
 
 # extra
 EXTRA_DIST =
diff --git a/configure.ac b/configure.ac
index b9bc770..c49e107 100644
--- a/configure.ac
+++ b/configure.ac
@@ -19,7 +19,6 @@ AH_TEMPLATE(IPv6, [IPv6])
 AH_TEMPLATE(NEED_ALIGN, [Alignment])
 AH_TEMPLATE(DISABLE_TLS, [Disable TLS encryption])
 AH_TEMPLATE(WITH_XKBFILE, [Use xkbfile for keyboard handling])
-AH_TEMPLATE(WITH_SCARD, [Define if smartcard is enabled])
 AH_TEMPLATE(WITH_DEBUG, [Turn on debugging messages])
 AH_TEMPLATE(WITH_DEBUG_GDI, [Turn on debugging messages])
 AH_TEMPLATE(WITH_DEBUG_NLA, [Turn on debugging messages])
@@ -158,10 +157,10 @@ fi
 # Smartcard
 #
 smartcard="no"
-AC_ARG_ENABLE(smartcard,
-	[AS_HELP_STRING([--enable-smartcard], [Enables smart-card support])],
+AC_ARG_WITH(smartcard,
+	[AS_HELP_STRING([--with-smartcard], [Build smart-card sub-plugin])],
 	[
-		if test "$enableval" = no; then
+		if test "$withval" = no; then
 			WITH_SCARD=0
 		else
 			case "$OSTYPE" in
@@ -178,16 +177,13 @@ AC_ARG_ENABLE(smartcard,
 			esac
 
 			if test x"$WITH_SCARD" = "x1"; then
-				CFLAGS="$CFLAGS $PCSCLITE_CFLAGS"
-				LIBS="$LIBS $PCSCLITE_LIBS"
+				AC_SUBST(PCSCLITE_CFLAGS)
+				AC_SUBST(PCSCLITE_LIBS)
 				smartcard="yes"
-				AC_DEFINE(WITH_SCARD)
 			fi
 		fi
 	])
 
-AM_CONDITIONAL(WITH_SCARD, test $WITH_SCARD)
-
 #
 # Alignment
 #
-- 
1.7.4.4

------------------------------------------------------------------------------
WhatsUp Gold - Download Free Network Management Software
The most intuitive, comprehensive, and cost-effective network 
management toolset available today.  Delivers lowest initial 
acquisition cost and overall TCO of any competing solution.
http://p.sf.net/sfu/whatsupgold-sd
_______________________________________________
Freerdp-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/freerdp-devel

Reply via email to