* [Qemu-devel] [PATCHv4 1/4] libcacard: s/strip(args++)/strip(args+1)
2011-07-22 11:42 [Qemu-devel] [PATCHv4 0/4] libcacard fixes Christophe Fergeau
@ 2011-07-22 11:42 ` Christophe Fergeau
2011-07-22 11:42 ` [Qemu-devel] [PATCHv4 2/4] libcacard: fix soft=... parsing in vcard_emul_options Christophe Fergeau
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Christophe Fergeau @ 2011-07-22 11:42 UTC (permalink / raw)
To: qemu-devel
vcard_emul_options used args = strip(args++) a few times, which
was not returning the expected result since the rest of the code
expected args to be increased by at least 1, which is not the case
if *args is not a blank space when this function is called.
Replace these calls by "strip(args+1)" which will do what we expect.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Reviewed-by: Alon Levy <alevy@redhat.com>
---
libcacard/vcard_emul_nss.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/libcacard/vcard_emul_nss.c b/libcacard/vcard_emul_nss.c
index f3db657..184252f 100644
--- a/libcacard/vcard_emul_nss.c
+++ b/libcacard/vcard_emul_nss.c
@@ -1041,7 +1041,7 @@ vcard_emul_options(const char *args)
args++;
continue;
}
- args = strip(args++);
+ args = strip(args+1);
type_params = args;
args = strpbrk(args + 1, ",)");
if (*args == 0) {
@@ -1052,7 +1052,7 @@ vcard_emul_options(const char *args)
continue;
}
type_params_length = args - name;
- args = strip(args++);
+ args = strip(args+1);
if (*args == 0) {
break;
}
--
1.7.6
^ permalink raw reply related [flat|nested] 6+ messages in thread* [Qemu-devel] [PATCHv4 2/4] libcacard: fix soft=... parsing in vcard_emul_options
2011-07-22 11:42 [Qemu-devel] [PATCHv4 0/4] libcacard fixes Christophe Fergeau
2011-07-22 11:42 ` [Qemu-devel] [PATCHv4 1/4] libcacard: s/strip(args++)/strip(args+1) Christophe Fergeau
@ 2011-07-22 11:42 ` Christophe Fergeau
2011-07-22 11:42 ` [Qemu-devel] [PATCHv4 3/4] libcacard: introduce NEXT_TOKEN macro Christophe Fergeau
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Christophe Fergeau @ 2011-07-22 11:42 UTC (permalink / raw)
To: qemu-devel
The previous parser had copy and paste errors when computing
vname_length and type_params_length, "name" was used instead
of respectively vname and type_params. This led to length that could
be bigger than the input string, and to access out of the array
bounds when trying to copy these strings. valgrind rightfully
complained about this. It also didn't handle empty fields correctly,
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Reviewed-by: Alon Levy <alevy@redhat.com>
---
libcacard/vcard_emul_nss.c | 45 +++++++++++++++++++++++++++----------------
1 files changed, 28 insertions(+), 17 deletions(-)
diff --git a/libcacard/vcard_emul_nss.c b/libcacard/vcard_emul_nss.c
index 184252f..9271f58 100644
--- a/libcacard/vcard_emul_nss.c
+++ b/libcacard/vcard_emul_nss.c
@@ -980,8 +980,6 @@ vcard_emul_options(const char *args)
{
int reader_count = 0;
VCardEmulOptions *opts;
- char type_str[100];
- int type_len;
/* Allow the future use of allocating the options structure on the fly */
memcpy(&options, &default_options, sizeof(options));
@@ -996,18 +994,24 @@ vcard_emul_options(const char *args)
* cert_2,cert_3...) */
if (strncmp(args, "soft=", 5) == 0) {
const char *name;
+ size_t name_length;
const char *vname;
+ size_t vname_length;
const char *type_params;
+ size_t type_params_length;
+ char type_str[100];
VCardEmulType type;
- int name_length, vname_length, type_params_length, count, i;
+ int count, i;
VirtualReaderOptions *vreaderOpt = NULL;
args = strip(args + 5);
if (*args != '(') {
continue;
}
+ args = strip(args+1);
+
name = args;
- args = strpbrk(args + 1, ",)");
+ args = strpbrk(args, ",)");
if (*args == 0) {
break;
}
@@ -1015,10 +1019,11 @@ vcard_emul_options(const char *args)
args++;
continue;
}
+ name_length = args - name;
args = strip(args+1);
- name_length = args - name - 2;
+
vname = args;
- args = strpbrk(args + 1, ",)");
+ args = strpbrk(args, ",)");
if (*args == 0) {
break;
}
@@ -1026,13 +1031,10 @@ vcard_emul_options(const char *args)
args++;
continue;
}
- vname_length = args - name - 2;
+ vname_length = args - vname;
args = strip(args+1);
- type_len = strpbrk(args, ",)") - args;
- assert(sizeof(type_str) > type_len);
- strncpy(type_str, args, type_len);
- type_str[type_len] = 0;
- type = vcard_emul_type_from_string(type_str);
+
+ type_params = args;
args = strpbrk(args, ",)");
if (*args == 0) {
break;
@@ -1041,9 +1043,16 @@ vcard_emul_options(const char *args)
args++;
continue;
}
+ type_params_length = args - type_params;
args = strip(args+1);
+
+ type_params_length = MIN(type_params_length, sizeof(type_str)-1);
+ strncpy(type_str, type_params, type_params_length);
+ type_str[type_params_length] = 0;
+ type = vcard_emul_type_from_string(type_str);
+
type_params = args;
- args = strpbrk(args + 1, ",)");
+ args = strpbrk(args, ",)");
if (*args == 0) {
break;
}
@@ -1051,8 +1060,9 @@ vcard_emul_options(const char *args)
args++;
continue;
}
- type_params_length = args - name;
+ type_params_length = args - type_params;
args = strip(args+1);
+
if (*args == 0) {
break;
}
@@ -1072,13 +1082,14 @@ vcard_emul_options(const char *args)
vreaderOpt->card_type = type;
vreaderOpt->type_params =
copy_string(type_params, type_params_length);
- count = count_tokens(args, ',', ')');
+ count = count_tokens(args, ',', ')') + 1;
vreaderOpt->cert_count = count;
vreaderOpt->cert_name = (char **)qemu_malloc(count*sizeof(char *));
for (i = 0; i < count; i++) {
- const char *cert = args + 1;
- args = strpbrk(args + 1, ",)");
+ const char *cert = args;
+ args = strpbrk(args, ",)");
vreaderOpt->cert_name[i] = copy_string(cert, args - cert);
+ args = strip(args+1);
}
if (*args == ')') {
args++;
--
1.7.6
^ permalink raw reply related [flat|nested] 6+ messages in thread* [Qemu-devel] [PATCHv4 3/4] libcacard: introduce NEXT_TOKEN macro
2011-07-22 11:42 [Qemu-devel] [PATCHv4 0/4] libcacard fixes Christophe Fergeau
2011-07-22 11:42 ` [Qemu-devel] [PATCHv4 1/4] libcacard: s/strip(args++)/strip(args+1) Christophe Fergeau
2011-07-22 11:42 ` [Qemu-devel] [PATCHv4 2/4] libcacard: fix soft=... parsing in vcard_emul_options Christophe Fergeau
@ 2011-07-22 11:42 ` Christophe Fergeau
2011-07-22 11:42 ` [Qemu-devel] [PATCHv4 4/4] libcacard: replace copy_string with strndup Christophe Fergeau
2011-07-22 12:15 ` [Qemu-devel] [PATCHv4 0/4] libcacard fixes Alon Levy
4 siblings, 0 replies; 6+ messages in thread
From: Christophe Fergeau @ 2011-07-22 11:42 UTC (permalink / raw)
To: qemu-devel
vcard_emul_options now has repetitive code to read the current
token and advance to the next. After the previous changes,
this repetitive code can be moved in a NEXT_TOKEN macro to
avoid having this code duplicated.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Reviewed-by: Alon Levy <alevy@redhat.com>
---
libcacard/vcard_emul_nss.c | 71 +++++++++++++++-----------------------------
1 files changed, 24 insertions(+), 47 deletions(-)
diff --git a/libcacard/vcard_emul_nss.c b/libcacard/vcard_emul_nss.c
index 9271f58..2a20bd6 100644
--- a/libcacard/vcard_emul_nss.c
+++ b/libcacard/vcard_emul_nss.c
@@ -975,6 +975,26 @@ find_blank(const char *str)
static VCardEmulOptions options;
#define READER_STEP 4
+/* Expects "args" to be at the beginning of a token (ie right after the ','
+ * ending the previous token), and puts the next token start in "token",
+ * and its length in "token_length". "token" will not be nul-terminated.
+ * After calling the macro, "args" will be advanced to the beginning of
+ * the next token.
+ * This macro may call continue or break.
+ */
+#define NEXT_TOKEN(token) \
+ (token) = args; \
+ args = strpbrk(args, ",)"); \
+ if (*args == 0) { \
+ break; \
+ } \
+ if (*args == ')') { \
+ args++; \
+ continue; \
+ } \
+ (token##_length) = args - (token); \
+ args = strip(args+1);
+
VCardEmulOptions *
vcard_emul_options(const char *args)
{
@@ -1010,58 +1030,15 @@ vcard_emul_options(const char *args)
}
args = strip(args+1);
- name = args;
- args = strpbrk(args, ",)");
- if (*args == 0) {
- break;
- }
- if (*args == ')') {
- args++;
- continue;
- }
- name_length = args - name;
- args = strip(args+1);
-
- vname = args;
- args = strpbrk(args, ",)");
- if (*args == 0) {
- break;
- }
- if (*args == ')') {
- args++;
- continue;
- }
- vname_length = args - vname;
- args = strip(args+1);
-
- type_params = args;
- args = strpbrk(args, ",)");
- if (*args == 0) {
- break;
- }
- if (*args == ')') {
- args++;
- continue;
- }
- type_params_length = args - type_params;
- args = strip(args+1);
-
+ NEXT_TOKEN(name)
+ NEXT_TOKEN(vname)
+ NEXT_TOKEN(type_params)
type_params_length = MIN(type_params_length, sizeof(type_str)-1);
strncpy(type_str, type_params, type_params_length);
type_str[type_params_length] = 0;
type = vcard_emul_type_from_string(type_str);
- type_params = args;
- args = strpbrk(args, ",)");
- if (*args == 0) {
- break;
- }
- if (*args == ')') {
- args++;
- continue;
- }
- type_params_length = args - type_params;
- args = strip(args+1);
+ NEXT_TOKEN(type_params)
if (*args == 0) {
break;
--
1.7.6
^ permalink raw reply related [flat|nested] 6+ messages in thread* [Qemu-devel] [PATCHv4 4/4] libcacard: replace copy_string with strndup
2011-07-22 11:42 [Qemu-devel] [PATCHv4 0/4] libcacard fixes Christophe Fergeau
` (2 preceding siblings ...)
2011-07-22 11:42 ` [Qemu-devel] [PATCHv4 3/4] libcacard: introduce NEXT_TOKEN macro Christophe Fergeau
@ 2011-07-22 11:42 ` Christophe Fergeau
2011-07-22 12:15 ` [Qemu-devel] [PATCHv4 0/4] libcacard fixes Alon Levy
4 siblings, 0 replies; 6+ messages in thread
From: Christophe Fergeau @ 2011-07-22 11:42 UTC (permalink / raw)
To: qemu-devel
copy_string reimplements strndup, this commit removes it and
replaces all copy_string uses with strndup.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Reviewed-by: Alon Levy <alevy@redhat.com>
---
libcacard/vcard_emul_nss.c | 23 ++++++-----------------
1 files changed, 6 insertions(+), 17 deletions(-)
diff --git a/libcacard/vcard_emul_nss.c b/libcacard/vcard_emul_nss.c
index 2a20bd6..de324ba 100644
--- a/libcacard/vcard_emul_nss.c
+++ b/libcacard/vcard_emul_nss.c
@@ -925,17 +925,6 @@ vcard_emul_replay_insertion_events(void)
/*
* Silly little functions to help parsing our argument string
*/
-static char *
-copy_string(const char *str, int str_len)
-{
- char *new_str;
-
- new_str = qemu_malloc(str_len+1);
- memcpy(new_str, str, str_len);
- new_str[str_len] = 0;
- return new_str;
-}
-
static int
count_tokens(const char *str, char token, char token_end)
{
@@ -1054,18 +1043,18 @@ vcard_emul_options(const char *args)
}
opts->vreader = vreaderOpt;
vreaderOpt = &vreaderOpt[opts->vreader_count];
- vreaderOpt->name = copy_string(name, name_length);
- vreaderOpt->vname = copy_string(vname, vname_length);
+ vreaderOpt->name = qemu_strndup(name, name_length);
+ vreaderOpt->vname = qemu_strndup(vname, vname_length);
vreaderOpt->card_type = type;
vreaderOpt->type_params =
- copy_string(type_params, type_params_length);
+ qemu_strndup(type_params, type_params_length);
count = count_tokens(args, ',', ')') + 1;
vreaderOpt->cert_count = count;
vreaderOpt->cert_name = (char **)qemu_malloc(count*sizeof(char *));
for (i = 0; i < count; i++) {
const char *cert = args;
args = strpbrk(args, ",)");
- vreaderOpt->cert_name[i] = copy_string(cert, args - cert);
+ vreaderOpt->cert_name[i] = qemu_strndup(cert, args - cert);
args = strip(args+1);
}
if (*args == ')') {
@@ -1092,7 +1081,7 @@ vcard_emul_options(const char *args)
args = strip(args+10);
params = args;
args = find_blank(args);
- opts->hw_type_params = copy_string(params, args-params);
+ opts->hw_type_params = qemu_strndup(params, args-params);
/* db="/data/base/path" */
} else if (strncmp(args, "db=", 3) == 0) {
const char *db;
@@ -1103,7 +1092,7 @@ vcard_emul_options(const char *args)
args++;
db = args;
args = strpbrk(args, "\"\n");
- opts->nss_db = copy_string(db, args-db);
+ opts->nss_db = qemu_strndup(db, args-db);
if (*args != 0) {
args++;
}
--
1.7.6
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCHv4 0/4] libcacard fixes
2011-07-22 11:42 [Qemu-devel] [PATCHv4 0/4] libcacard fixes Christophe Fergeau
` (3 preceding siblings ...)
2011-07-22 11:42 ` [Qemu-devel] [PATCHv4 4/4] libcacard: replace copy_string with strndup Christophe Fergeau
@ 2011-07-22 12:15 ` Alon Levy
4 siblings, 0 replies; 6+ messages in thread
From: Alon Levy @ 2011-07-22 12:15 UTC (permalink / raw)
To: Christophe Fergeau; +Cc: qemu-devel
On Fri, Jul 22, 2011 at 01:42:16PM +0200, Christophe Fergeau wrote:
> Hi,
>
I'll add all of these to the pull request, thanks.
> This is a resend of my series of libcacard fixes, with the correct patches this
> time.
>
> Changes since v3:
> - the series now includes the intended patches
>
> Changes since v2:
> - added Reviewed-by: Alon Levy <alevy@redhat.com> to the 4 patches
>
> Changes since v1:
> - split first patch into 3 patches, v1 2/2 is unchanged
>
> Christophe Fergeau (4):
> libcacard: fix soft=... parsing in vcard_emul_options
> libcacard: introduce NEXT_TOKEN macro
> libcacard: replace copy_string with strndup
> libcacard: don't leak vcard_emul_alloc_arrays mem
>
> libcacard/vcard_emul_nss.c | 124 +++++++++++++++++++------------------------
> 1 files changed, 55 insertions(+), 69 deletions(-)
>
> --
> 1.7.6
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread