From 510e1b802747aa6d1eb1b078ed64f1de3caa845d Mon Sep 17 00:00:00 2001
From: Holger Dengler <hd@linux.vnet.ibm.com>
Date: Fri, 17 Jun 2011 11:14:56 +0200
Subject: [PATCH 1/3] Description: libica: Fix temporary buffer allocation in
ica_get_version(). Symptom: Sometimes
calling ica_get_version() ends up in a
segmentation fault. Problem: In function
ica_get_version() the version information
is copied to a temporary buffer. The allocated buffer
is to small to hold the complete version
information and the trailing termination
character. Solution: Adjust the allocation of the
temporary buffer as large as the version
information plus the string terminator.
---
src/ica_api.c | 59 ++++++++++++++++++++++++++++++--------------------------
1 files changed, 32 insertions(+), 27 deletions(-)
diff --git a/src/ica_api.c b/src/ica_api.c
index a853ce1..e44678a 100644
--- a/src/ica_api.c
+++ b/src/ica_api.c
@@ -36,6 +36,8 @@
#define DEFAULT2_CRYPT_DEVICE "/dev/z90crypt"
#define DEFAULT3_CRYPT_DEVICE "/dev/zcrypt"
+#define MAX_VERSION_LENGTH 16
+
static unsigned int check_des_parms(unsigned int mode,
unsigned long data_length,
const unsigned char *in_data,
@@ -1044,48 +1046,51 @@ unsigned int ica_aes_cmac(const unsigned char *message, unsigned long message_le
unsigned int ica_get_version(libica_version_info *version_info)
{
- /*
- * We expect the libica version information in the format x.y.z
- * defined in the macro VERSION as part of the build process.
- */
-#ifndef VERSION
- return EIO;
-#endif
-
- int length = strlen(VERSION);
+#ifdef VERSION
+ int length;
int rc;
- int i = 1;
+ int i;
char *pch;
+ char *saveptr;
if (version_info == NULL) {
return EINVAL;
}
- char buffer[length];
- rc = sprintf(buffer, "%s", VERSION);
+ length = strnlen(VERSION, MAX_VERSION_LENGTH);
+ char buffer[length+1];
+
+ rc = snprintf(buffer, (length+1), "%s", VERSION);
if (rc <= 0) {
- return 1;
+ return EIO;
}
- pch = strtok(buffer, ".");
-
- while (pch != NULL) {
- if (i == 1)
+ for (pch = strtok_r(buffer, ".", &saveptr), i = 1;
+ pch != NULL;
+ pch = strtok_r(NULL, ".", &saveptr), i++)
+ {
+ switch(i) {
+ case 1:
version_info->major_version = atoi(pch);
- if (i == 2)
+ break;
+ case 2:
version_info->minor_version = atoi(pch);
- if (i == 3)
+ break;
+ case 3:
version_info->fixpack_version = atoi(pch);
- if (i > 3)
- return 1;
-
- pch = strtok(NULL, ".");
- i++;
+ break;
+ default:
+ return EIO;
+ }
}
- if (i < 3) {
- return 1;
- }
+ if (i < 3)
+ return EIO;
return 0;
+#else
+ /* We expect the libica version information in the format x.y.z
+ * defined in the macro VERSION as part of the build process. */
+ return EIO;
+#endif
}
--
1.7.4.4
From cc1b8bcfe9dd4a119e726380626b372175595980 Mon Sep 17 00:00:00 2001
From: Holger Dengler <hd@linux.vnet.ibm.com>
Date: Fri, 17 Jun 2011 11:20:27 +0200
Subject: [PATCH 2/3] Fix result/error handling in testcase for
ica_get_version().
---
src/tests/libica_get_version.c | 46 +++++++++++++++++++++++----------------
1 files changed, 27 insertions(+), 19 deletions(-)
diff --git a/src/tests/libica_get_version.c b/src/tests/libica_get_version.c
index 086e228..63f8c55 100644
--- a/src/tests/libica_get_version.c
+++ b/src/tests/libica_get_version.c
@@ -14,37 +14,45 @@
*/
#include <stdio.h>
#include <stdlib.h>
+#include <errno.h>
#include "ica_api.h"
int main(int argc, char **argv)
{
libica_version_info version_info;
int rc;
+ int failed = 0;
- printf("Testing libica API ica_get_version(). Invalid input (NULL).\n");
-
+ printf("Testing libica API ica_get_version() w/ invalid input (NULL).\n");
rc = ica_get_version(NULL);
-
- if (rc == 0) {
- printf("OK. Not expected (RC=%d).\n", rc);
- } else {
- printf("Error. Expected(RC=%d).\n", rc);
+ if (rc == EINVAL)
+ printf("Test successful");
+ else {
+ printf("Test failed");
+ failed++;
}
+ printf(" (rc=%d, expected: %d)\n", rc, EINVAL);
- printf("Testing libica API ica_get_version_(). Valid input.\n");
-
+ printf("Testing libica API ica_get_version_() w/ valid input.\n");
rc = ica_get_version(&version_info);
-
- if (rc == 0) {
- printf("OK. Expected (RC=%d).\n", rc);
- } else {
- printf("Error. Not expected (RC=%d).\n", rc);
- return rc;
+ if (rc == 0)
+ printf("Test successful");
+ else {
+ printf("Test failed");
+ failed++;
}
+ printf(" (rc=%d, expected: %d)\n", rc, 0);
- printf("Major_version:%d\n", version_info.major_version);
- printf("Minor_version:%d\n", version_info.minor_version);
- printf("Fixpack_version:%d\n", version_info.fixpack_version);
+ printf("Major_version:%d, minor_version %d, fixpack_version %d\n",
+ version_info.major_version,
+ version_info.minor_version,
+ version_info.fixpack_version);
- return 0;
+ if (failed) {
+ printf("Failed tests: %d\n", failed);
+ return 1;
+ } else {
+ printf("All tests completed sucessfully\n");
+ return 0;
+ }
}
--
1.7.4.4
From 51474af2b74735931e89864c392a407499f2f3a6 Mon Sep 17 00:00:00 2001
From: Holger Dengler <hd@linux.vnet.ibm.com>
Date: Thu, 30 Jun 2011 12:46:24 +0200
Subject: [PATCH 3/3] [PATCH]: synchronize shared memory ref counting
Description: libica: synchronize shared memory reference counting
for library statisitcs
Symptom: sshd daemon blocks all login tries. Other processes,
which accessing libica, do not run correctly.
Problem: Libica setup a shared memory for counting some
statistics across all processes and threads using the
library. The locking and reference counting is re-
ordered by compiler optimization. Beside that, the lock
is not released if the last process unloads the library.
Solution: Insert barriers to synchronize shared memory reference
counting. Release lock, if last process unloads the
library. Use non-blocking lock, which may have the
side effect, that on locking problems the statistics
counting is disabled instead of blocking processes,
which using libica.
---
src/icastats_shared.c | 67 +++++++++++++++++++++++++++++++++++--------------
1 files changed, 48 insertions(+), 19 deletions(-)
diff --git a/src/icastats_shared.c b/src/icastats_shared.c
index b044dea..ead1067 100644
--- a/src/icastats_shared.c
+++ b/src/icastats_shared.c
@@ -6,6 +6,7 @@
/**
* Authors: Christian Maaser <cmaaser@de.ibm.com>
+ * Holger Dengler <hd@linux.vnet.ibm.com>
*
* Copyright IBM Corp. 2009, 2011
*/
@@ -23,9 +24,11 @@ typedef struct statis_entry {
uint32_t software;
} stats_entry_t;
-stats_entry_t *stats = 0;
-int *stats_ref_counter = 0;
-int stats_shm_handle = 0;
+#define NOT_INITIALIZED (-1)
+
+stats_entry_t *stats = NULL;
+volatile int *stats_ref_counter = NULL;
+volatile int stats_shm_handle = NOT_INITIALIZED;
void atomic_add(int *x, int i)
{
@@ -43,24 +46,32 @@ void atomic_add(int *x, int i)
int stats_mmap()
{
+ int local_stats_shm_handle;
/* Use flock to avoid races between open and close of shm by different
* processes. Put reference counter into shm to check how much
* processes are accesing the shm. Additionaly a global and a local
* handle for the shm are used to prevend different threads from
* overriding their shm handle one another.
+ * Non-blocking flocks are used. This may end-up in disabled statistics
+ * instead of locking each process using libica.
*/
- if (!stats) {
- int local_stats_shm_handle = shm_open(STATS_SHM_ID, O_CREAT | O_RDWR,
- S_IRUSR | S_IWUSR | S_IRGRP |
- S_IWGRP | S_IROTH | S_IWOTH);
- if (local_stats_shm_handle == -1)
+ if (stats == NULL) {
+ local_stats_shm_handle = shm_open(STATS_SHM_ID, O_CREAT | O_RDWR,
+ S_IRUSR | S_IWUSR | S_IRGRP |
+ S_IWGRP | S_IROTH | S_IWOTH);
+ if (local_stats_shm_handle == NOT_INITIALIZED)
return -1;
if (ftruncate(local_stats_shm_handle, STATS_SHM_SIZE) == -1)
return -1;
-
- if (flock(local_stats_shm_handle, LOCK_EX) == -1)
+ if (flock(local_stats_shm_handle, LOCK_EX | LOCK_NB) == -1)
return -1;
- if (stats_shm_handle != 0) {
+
+ /* This barrier prohibits re-ordering of locking and
+ * reference counting due to compiler optimizations. */
+ asm volatile ("" : : : "memory");
+
+ if (stats_shm_handle != NOT_INITIALIZED) {
+ /* Another process/thread won the race, give up. */
flock(local_stats_shm_handle, LOCK_UN);
return 0;
}
@@ -68,17 +79,22 @@ int stats_mmap()
PROT_WRITE, MAP_SHARED,
local_stats_shm_handle, 0);
if (stats_ref_counter == MAP_FAILED) {
- stats_ref_counter = 0;
+ stats_ref_counter = NULL;
flock(local_stats_shm_handle, LOCK_UN);
return -1;
}
++(*stats_ref_counter);
- stats = (stats_entry_t *) (stats_ref_counter + 1);
+ stats = (stats_entry_t *) (stats_ref_counter + sizeof(stats_ref_counter));
stats_shm_handle = local_stats_shm_handle;
flock(local_stats_shm_handle, LOCK_UN);
} else {
- if (flock(stats_shm_handle, LOCK_EX) == -1)
+ if (flock(stats_shm_handle, LOCK_EX | LOCK_NB) == -1)
return -1;
+
+ /* This barrier prohibits re-ordering of locking and
+ * reference counting due to compiler optimizations. */
+ asm volatile ("" : : : "memory");
+
++(*stats_ref_counter);
flock(stats_shm_handle, LOCK_UN);
}
@@ -88,13 +104,22 @@ int stats_mmap()
void stats_munmap()
{
- if (!stats)
+ int tmp_handle;
+ if (stats == NULL)
return;
- if (flock(stats_shm_handle, LOCK_EX) == -1)
+ if (flock(stats_shm_handle, LOCK_EX | LOCK_NB) == -1)
return;
+
+ /* This barrier prohibits re-ordering of locking and
+ * reference counting due to compiler optimizations. */
+ asm volatile ("" : : : "memory");
+
if (--(*stats_ref_counter) == 0) {
- munmap(stats_ref_counter, STATS_SHM_SIZE);
+ munmap((void *)stats_ref_counter, STATS_SHM_SIZE);
+ tmp_handle = stats_shm_handle;
+ stats_shm_handle = NOT_INITIALIZED;
+ flock(tmp_handle, LOCK_UN);
shm_unlink(STATS_SHM_ID);
stats = 0;
}
@@ -104,7 +129,7 @@ void stats_munmap()
uint32_t stats_query(stats_fields_t field, int hardware)
{
- if (!stats)
+ if (stats == NULL)
return 0;
if (hardware)
@@ -115,7 +140,7 @@ uint32_t stats_query(stats_fields_t field, int hardware)
void stats_increment(stats_fields_t field, int hardware)
{
- if (!stats)
+ if (stats == NULL)
return;
if (hardware)
@@ -127,6 +152,10 @@ void stats_increment(stats_fields_t field, int hardware)
void stats_reset()
{
unsigned int i;
+
+ if (stats == NULL)
+ return;
+
for (i = 0; i != ICA_NUM_STATS; ++i) {
stats[i].hardware = 0;
stats[i].software = 0;
--
1.7.4.4