[PATCH v2 1/5] cipher: Update for current kernel akcipher interface
by Mat Martineau
There are some significant differences in the current iteration of
kernel AF_ALG akcipher support:
* Kernel handles padding
* Kernel accepts DER-encoded certs as-is (no need to extract values)
* Must explicitly set private or public key
---
ell/cipher-private.h | 3 -
ell/cipher.c | 302 +++++++++++++--------------------------------------
ell/tls.c | 43 +++-----
3 files changed, 86 insertions(+), 262 deletions(-)
diff --git a/ell/cipher-private.h b/ell/cipher-private.h
index 7d115f6..77ddd06 100644
--- a/ell/cipher-private.h
+++ b/ell/cipher-private.h
@@ -20,9 +20,6 @@
*
*/
-uint8_t *extract_rsakey(uint8_t *pkcs1_key, size_t pkcs1_key_len,
- size_t *out_len);
-
#define ASN1_ID(class, pc, tag) (((class) << 6) | ((pc) << 5) | (tag))
#define ASN1_CLASS_UNIVERSAL 0
diff --git a/ell/cipher.c b/ell/cipher.c
index 671071b..f637767 100644
--- a/ell/cipher.c
+++ b/ell/cipher.c
@@ -78,6 +78,10 @@ struct af_alg_iv {
#define SOL_ALG 279
#endif
+#ifndef ALG_SET_PUBKEY
+#define ALG_SET_PUBKEY 6
+#endif
+
#define is_valid_type(type) ((type) <= L_CIPHER_DES3_EDE_CBC)
struct l_cipher {
@@ -87,10 +91,11 @@ struct l_cipher {
};
static int create_alg(const char *alg_type, const char *alg_name,
- const void *key, size_t key_length)
+ const void *key, size_t key_length, bool public)
{
struct sockaddr_alg salg;
int sk;
+ int keyopt;
int ret;
sk = socket(PF_ALG, SOCK_SEQPACKET | SOCK_CLOEXEC, 0);
@@ -107,7 +112,8 @@ static int create_alg(const char *alg_type, const char *alg_name,
return -1;
}
- if (setsockopt(sk, SOL_ALG, ALG_SET_KEY, key, key_length) < 0) {
+ keyopt = public ? ALG_SET_PUBKEY : ALG_SET_KEY;
+ if (setsockopt(sk, SOL_ALG, keyopt, key, key_length) < 0) {
close(sk);
return -1;
}
@@ -149,11 +155,13 @@ LIB_EXPORT struct l_cipher *l_cipher_new(enum l_cipher_type type,
break;
}
- cipher->encrypt_sk = create_alg("skcipher", alg_name, key, key_length);
+ cipher->encrypt_sk = create_alg("skcipher", alg_name, key, key_length,
+ false);
if (cipher->encrypt_sk < 0)
goto error_free;
- cipher->decrypt_sk = create_alg("skcipher", alg_name, key, key_length);
+ cipher->decrypt_sk = create_alg("skcipher", alg_name, key, key_length,
+ false);
if (cipher->decrypt_sk < 0)
goto error_close;
@@ -178,7 +186,8 @@ LIB_EXPORT void l_cipher_free(struct l_cipher *cipher)
}
static bool operate_cipher(int sk, __u32 operation,
- const void *in, void *out, size_t len)
+ const void *in, void *out, size_t len_in,
+ size_t len_out)
{
char c_msg_buf[CMSG_SPACE(sizeof(operation))];
struct msghdr msg;
@@ -198,7 +207,7 @@ static bool operate_cipher(int sk, __u32 operation,
memcpy(CMSG_DATA(c_msg), &operation, sizeof(operation));
iov.iov_base = (void *) in;
- iov.iov_len = len;
+ iov.iov_len = len_in;
msg.msg_iov = &iov;
msg.msg_iovlen = 1;
@@ -206,7 +215,7 @@ static bool operate_cipher(int sk, __u32 operation,
if (sendmsg(sk, &msg, 0) < 0)
return false;
- if (read(sk, out, len) < 0)
+ if (read(sk, out, len_out) < 0)
return false;
return true;
@@ -221,7 +230,8 @@ LIB_EXPORT bool l_cipher_encrypt(struct l_cipher *cipher,
if (unlikely(!in) || unlikely(!out))
return false;
- return operate_cipher(cipher->encrypt_sk, ALG_OP_ENCRYPT, in, out, len);
+ return operate_cipher(cipher->encrypt_sk, ALG_OP_ENCRYPT, in, out, len,
+ len);
}
LIB_EXPORT bool l_cipher_decrypt(struct l_cipher *cipher,
@@ -233,7 +243,8 @@ LIB_EXPORT bool l_cipher_decrypt(struct l_cipher *cipher,
if (unlikely(!in) || unlikely(!out))
return false;
- return operate_cipher(cipher->decrypt_sk, ALG_OP_DECRYPT, in, out, len);
+ return operate_cipher(cipher->decrypt_sk, ALG_OP_DECRYPT, in, out, len,
+ len);
}
LIB_EXPORT bool l_cipher_set_iv(struct l_cipher *cipher, const uint8_t *iv,
@@ -275,6 +286,7 @@ LIB_EXPORT bool l_cipher_set_iv(struct l_cipher *cipher, const uint8_t *iv,
struct l_asymmetric_cipher {
struct l_cipher cipher;
int key_size;
+ bool public_key;
};
static inline int parse_asn1_definite_length(const uint8_t **buf,
@@ -334,20 +346,31 @@ static bool parse_rsa_key(struct l_asymmetric_cipher *cipher, const void *key,
* and cache the size of the modulus n for later use.
* (RFC3279)
*/
+ size_t seq_length;
size_t n_length;
+ uint8_t *seq;
uint8_t *der;
uint8_t tag;
+ bool pubkey = true;
if (key_length < 8)
return false;
/* Unpack the outer SEQUENCE */
- der = der_find_elem((uint8_t *) key, key_length, 0, &tag, &n_length);
- if (!der || tag != ASN1_ID_SEQUENCE)
+ seq = der_find_elem((uint8_t *) key, key_length, 0, &tag, &seq_length);
+ if (!seq || tag != ASN1_ID_SEQUENCE)
return false;
- /* Take first INTEGER as the modulus */
- der = der_find_elem(der, n_length, 0, &tag, &n_length);
+ /* First INTEGER may be a 1-byte version (for private key) or
+ * the modulus (public key)
+ */
+ der = der_find_elem(seq, seq_length, 0, &tag, &n_length);
+ if (der && tag == ASN1_ID_INTEGER && n_length == 1) {
+ /* Found version number, implies this is a private key. */
+ der = der_find_elem(seq, seq_length, 1, &tag, &n_length);
+ pubkey = false;
+ }
+
if (!der || tag != ASN1_ID_INTEGER || n_length < 4)
return false;
@@ -358,95 +381,11 @@ static bool parse_rsa_key(struct l_asymmetric_cipher *cipher, const void *key,
}
cipher->key_size = n_length;
+ cipher->public_key = pubkey;
return true;
}
-static void write_asn1_definite_length(uint8_t **buf, size_t len)
-{
- int n;
-
- if (len < 0x80) {
- *(*buf)++ = len;
-
- return;
- }
-
- for (n = 1; len >> (n * 8); n++);
- *(*buf)++ = 0x80 | n;
-
- while (n--)
- *(*buf)++ = len >> (n * 8);
-}
-
-/*
- * Extract a ASN1 RsaKey-formatted public+private key structure in the
- * form used in the kernel. It is simpler than the PKCS#1 form as it only
- * contains the N, E and D integers and also correctly parses as a PKCS#1
- * RSAPublicKey.
- */
-uint8_t *extract_rsakey(uint8_t *pkcs1_key, size_t pkcs1_key_len,
- size_t *out_len)
-{
- uint8_t *key, *ptr, *ver, *n, *e, *d;
- uint8_t tag;
- size_t ver_len, n_len, e_len, d_len;
- int pos;
-
- /* Unpack the outer SEQUENCE */
- pkcs1_key = der_find_elem(pkcs1_key, pkcs1_key_len, 0, &tag,
- &pkcs1_key_len);
- if (!pkcs1_key || tag != ASN1_ID_SEQUENCE)
- return NULL;
-
- /* Check if the version element if present */
- ver = der_find_elem(pkcs1_key, pkcs1_key_len, 0, &tag, &ver_len);
- if (!ver || tag != ASN1_ID_INTEGER)
- return NULL;
-
- pos = (ver_len == 1 && ver[0] == 0x00) ? 1 : 0;
-
- n = der_find_elem(pkcs1_key, pkcs1_key_len, pos + 0, &tag, &n_len);
- if (!n || tag != ASN1_ID_INTEGER)
- return NULL;
-
- e = der_find_elem(pkcs1_key, pkcs1_key_len, pos + 1, &tag, &e_len);
- if (!e || tag != ASN1_ID_INTEGER)
- return NULL;
-
- d = der_find_elem(pkcs1_key, pkcs1_key_len, pos + 2, &tag, &d_len);
- if (!d || tag != ASN1_ID_INTEGER)
- return NULL;
-
- /* New SEQUENCE length including tags and lengths */
- *out_len = 1 + (n_len >= 0x80 ? n_len >= 0x100 ? 3 : 2 : 1) + n_len +
- 1 + (e_len >= 0x80 ? e_len >= 0x100 ? 3 : 2 : 1) + e_len +
- 1 + (d_len >= 0x80 ? d_len >= 0x100 ? 3 : 2 : 1) + d_len;
- ptr = key = l_malloc(*out_len +
- 1 + (*out_len >= 0x80 ? *out_len >= 0x100 ? 3 : 2 : 1));
-
- *ptr++ = ASN1_ID_SEQUENCE;
- write_asn1_definite_length(&ptr, *out_len);
-
- *ptr++ = ASN1_ID_INTEGER;
- write_asn1_definite_length(&ptr, n_len);
- memcpy(ptr, n, n_len);
- ptr += n_len;
-
- *ptr++ = ASN1_ID_INTEGER;
- write_asn1_definite_length(&ptr, e_len);
- memcpy(ptr, e, e_len);
- ptr += e_len;
-
- *ptr++ = ASN1_ID_INTEGER;
- write_asn1_definite_length(&ptr, d_len);
- memcpy(ptr, d, d_len);
- ptr += d_len;
-
- *out_len = ptr - key;
- return key;
-}
-
LIB_EXPORT struct l_asymmetric_cipher *l_asymmetric_cipher_new(
enum l_asymmetric_cipher_type type,
const void *key, size_t key_length)
@@ -468,19 +407,23 @@ LIB_EXPORT struct l_asymmetric_cipher *l_asymmetric_cipher_new(
if (!parse_rsa_key(cipher, key, key_length))
goto error_free;
- alg_name = "rsa";
+ alg_name = "pkcs1pad(rsa)";
break;
}
cipher->cipher.encrypt_sk = create_alg("akcipher", alg_name,
- key, key_length);
+ key, key_length,
+ cipher->public_key);
if (cipher->cipher.encrypt_sk < 0)
goto error_free;
- cipher->cipher.decrypt_sk = create_alg("akcipher", alg_name,
- key, key_length);
- if (cipher->cipher.decrypt_sk < 0)
- goto error_close;
+ if (!cipher->public_key) {
+ cipher->cipher.decrypt_sk = create_alg("akcipher", alg_name,
+ key, key_length,
+ cipher->public_key);
+ if (cipher->cipher.decrypt_sk < 0)
+ goto error_close;
+ }
return cipher;
@@ -508,151 +451,52 @@ LIB_EXPORT int l_asymmetric_cipher_get_key_size(
return cipher->key_size;
}
-static void getrandom_nonzero(uint8_t *buf, int len)
-{
- while (len--) {
- l_getrandom(buf, 1);
- while (buf[0] == 0)
- l_getrandom(buf, 1);
-
- buf++;
- }
-}
-
-LIB_EXPORT bool l_asymmetric_cipher_encrypt(struct l_asymmetric_cipher *cipher,
+LIB_EXPORT bool l_asymmetric_cipher_encrypt(struct l_asymmetric_cipher *acipher,
const void *in, void *out,
size_t len_in, size_t len_out)
{
- if (cipher->cipher.type == L_CIPHER_RSA_PKCS1_V1_5) {
- /* PKCS#1 v1.5 RSA padding according to RFC3447 */
- uint8_t buf[cipher->key_size];
- int ps_len = cipher->key_size - len_in - 3;
-
- if (len_in > (size_t) cipher->key_size - 11)
- return false;
- if (len_out != (size_t) cipher->key_size)
- return false;
-
- buf[0] = 0x00;
- buf[1] = 0x02;
- getrandom_nonzero(buf + 2, ps_len);
- buf[ps_len + 2] = 0x00;
- memcpy(buf + ps_len + 3, in, len_in);
-
- if (!l_cipher_encrypt(&cipher->cipher, buf, out,
- cipher->key_size))
- return false;
- }
+ if (unlikely(!acipher))
+ return false;
- return true;
+ if (unlikely(!in) || unlikely(!out))
+ return false;
+
+ return operate_cipher(acipher->cipher.encrypt_sk, ALG_OP_ENCRYPT,
+ in, out, len_in, len_out);
}
-LIB_EXPORT bool l_asymmetric_cipher_decrypt(struct l_asymmetric_cipher *cipher,
+LIB_EXPORT bool l_asymmetric_cipher_decrypt(struct l_asymmetric_cipher *acipher,
const void *in, void *out,
size_t len_in, size_t len_out)
{
- if (cipher->cipher.type == L_CIPHER_RSA_PKCS1_V1_5) {
- /* PKCS#1 v1.5 RSA padding according to RFC3447 */
- uint8_t buf[cipher->key_size];
- int pos;
-
- if (len_in != (size_t) cipher->key_size)
- return false;
-
- if (!l_cipher_decrypt(&cipher->cipher, in, buf,
- cipher->key_size))
- return false;
-
- if (buf[0] != 0x00)
- return false;
- if (buf[1] != 0x02)
- return false;
-
- for (pos = 2; pos < cipher->key_size; pos++)
- if (buf[pos] == 0)
- break;
- if (pos < 10 || pos == cipher->key_size)
- return false;
-
- pos++;
- if (len_out != (size_t) cipher->key_size - pos)
- return false;
-
- memcpy(out, buf + pos, cipher->key_size - pos);
- }
+ if (unlikely(!acipher))
+ return false;
- return true;
+ if (unlikely(!in) || unlikely(!out))
+ return false;
+
+ return operate_cipher(acipher->cipher.decrypt_sk, ALG_OP_DECRYPT,
+ in, out, len_in, len_out);
}
LIB_EXPORT bool l_asymmetric_cipher_sign(struct l_asymmetric_cipher *cipher,
const void *in, void *out,
size_t len_in, size_t len_out)
{
- if (cipher->cipher.type == L_CIPHER_RSA_PKCS1_V1_5) {
- /* PKCS#1 v1.5 RSA padding according to RFC3447 */
- uint8_t buf[cipher->key_size];
- int ps_len = cipher->key_size - len_in - 3;
-
- if (len_in > (size_t) cipher->key_size - 11)
- return false;
- if (len_out != (size_t) cipher->key_size)
- return false;
-
- buf[0] = 0x00;
- buf[1] = 0x01;
- memset(buf + 2, 0xff, ps_len);
- buf[ps_len + 2] = 0x00;
- memcpy(buf + ps_len + 3, in, len_in);
-
- /*
- * The RSA signing operation uses the same primitive as
- * decryption so just call decrypt for now.
- */
- if (!l_cipher_decrypt(&cipher->cipher, buf, out,
- cipher->key_size))
- return false;
- }
-
- return true;
+ /*
+ * The RSA signing operation uses the same primitive as
+ * decryption so just call decrypt for now.
+ */
+ return l_asymmetric_cipher_decrypt(cipher, in, out, len_in, len_out);
}
LIB_EXPORT bool l_asymmetric_cipher_verify(struct l_asymmetric_cipher *cipher,
const void *in, void *out,
size_t len_in, size_t len_out)
{
- if (cipher->cipher.type == L_CIPHER_RSA_PKCS1_V1_5) {
- /* PKCS#1 v1.5 RSA padding according to RFC3447 */
- uint8_t buf[cipher->key_size];
- int pos;
-
- if (len_in != (size_t) cipher->key_size)
- return false;
-
- /*
- * The RSA verify operation uses the same primitive as
- * encryption so just call encrypt.
- */
- if (!l_cipher_encrypt(&cipher->cipher, in, buf,
- cipher->key_size))
- return false;
-
- if (buf[0] != 0x00)
- return false;
- if (buf[1] != 0x01)
- return false;
-
- for (pos = 2; pos < cipher->key_size; pos++)
- if (buf[pos] != 0xff)
- break;
- if (pos < 10 || pos == cipher->key_size || buf[pos] != 0)
- return false;
-
- pos++;
- if (len_out != (size_t) cipher->key_size - pos)
- return false;
-
- memcpy(out, buf + pos, cipher->key_size - pos);
- }
-
- return true;
+ /*
+ * The RSA verify operation uses the same primitive as
+ * encryption so just call encrypt.
+ */
+ return l_asymmetric_cipher_encrypt(cipher, in, out, len_in, len_out);
}
diff --git a/ell/tls.c b/ell/tls.c
index a20236f..a55f24c 100644
--- a/ell/tls.c
+++ b/ell/tls.c
@@ -920,8 +920,8 @@ static bool tls_rsa_sign(struct l_tls *tls, uint8_t **out,
tls_get_hash_t get_hash)
{
struct l_asymmetric_cipher *rsa_privkey;
- uint8_t *privkey, *privkey_short;
- size_t key_size, short_size;
+ uint8_t *privkey;
+ size_t key_size;
bool result;
const struct tls_hash_algorithm *hash_type;
uint8_t hash[HANDSHAKE_HASH_MAX_SIZE];
@@ -945,19 +945,9 @@ static bool tls_rsa_sign(struct l_tls *tls, uint8_t **out,
return false;
}
- privkey_short = extract_rsakey(privkey, key_size, &short_size);
- tls_free_key(privkey, key_size);
-
- if (!privkey_short) {
- tls_disconnect(tls, TLS_ALERT_INTERNAL_ERROR,
- TLS_ALERT_BAD_CERT);
-
- return false;
- }
-
rsa_privkey = l_asymmetric_cipher_new(L_CIPHER_RSA_PKCS1_V1_5,
- privkey_short, short_size);
- tls_free_key(privkey_short, short_size);
+ privkey, key_size);
+ tls_free_key(privkey, key_size);
if (!rsa_privkey) {
tls_disconnect(tls, TLS_ALERT_INTERNAL_ERROR, 0);
@@ -1080,11 +1070,14 @@ static bool tls_rsa_verify(struct l_tls *tls, const uint8_t *in, size_t len,
*/
}
- digest_info = alloca(expected_len);
+ if (expected_len > key_size)
+ goto err_free_rsa;
+
+ digest_info = alloca(key_size);
result = l_asymmetric_cipher_verify(rsa_client_pubkey, in + 4,
digest_info,
- key_size, expected_len);
+ key_size, key_size);
l_asymmetric_cipher_free(rsa_client_pubkey);
@@ -1743,8 +1736,8 @@ static void tls_handle_rsa_client_key_xchg(struct l_tls *tls,
{
uint8_t pre_master_secret[48], random_secret[46];
struct l_asymmetric_cipher *rsa_server_privkey;
- uint8_t *privkey, *privkey_short;
- size_t key_size, short_size;
+ uint8_t *privkey;
+ size_t key_size;
bool result;
if (!tls->priv_key_path) {
@@ -1764,19 +1757,9 @@ static void tls_handle_rsa_client_key_xchg(struct l_tls *tls,
return;
}
- privkey_short = extract_rsakey(privkey, key_size, &short_size);
- tls_free_key(privkey, key_size);
-
- if (!privkey_short) {
- tls_disconnect(tls, TLS_ALERT_INTERNAL_ERROR,
- TLS_ALERT_BAD_CERT);
-
- return;
- }
-
rsa_server_privkey = l_asymmetric_cipher_new(L_CIPHER_RSA_PKCS1_V1_5,
- privkey_short, short_size);
- tls_free_key(privkey_short, short_size);
+ privkey, key_size);
+ tls_free_key(privkey, key_size);
if (!rsa_server_privkey) {
tls_disconnect(tls, TLS_ALERT_INTERNAL_ERROR, 0);
--
2.8.4
6 years, 2 months
[PATCH 01/11] gitignore: Add unit/test-dbus-message-fds
by Mat Martineau
---
.gitignore | 1 +
1 file changed, 1 insertion(+)
diff --git a/.gitignore b/.gitignore
index 6e35752..cad1672 100644
--- a/.gitignore
+++ b/.gitignore
@@ -36,6 +36,7 @@ unit/test-genl-msg
unit/test-kdbus
unit/test-dbus
unit/test-dbus-message
+unit/test-dbus-message-fds
unit/test-dbus-util
unit/test-dbus-service
unit/test-dbus-watch
--
2.8.3
6 years, 2 months
[PATCH v2 1/3] main: Manually initialize and clean up the main loop
by Mat Martineau
Atomatic initialization and cleanup of the main loop created some
issues with orderly shutdown of an ELL program. If cleanup functions
were called after the l_main_run() returned, they could indirectly set
up the epoll file descriptor and associated resources again. Those
resources were not freed after the cleanup functions ran.
Now, epoll resources are set up by l_main_init() and cleaned up by
l_main_exit(). If cleanup functions set up any idles or watches
(including watch-based timeouts, signals, or ios) after l_main_run()
returns, those will be destroyed by l_main_exit().
l_main_init() must be called before l_main_run() or any other ELL
function that expects epoll to be set up. l_main_exit() should be
called after l_main_run() returns to ensure that all resources are
freed.
---
ell/main.c | 87 ++++++++++++++++++++++++++++++++++++++------------------------
ell/main.h | 4 ++-
2 files changed, 57 insertions(+), 34 deletions(-)
diff --git a/ell/main.c b/ell/main.c
index f53ed9e..882690c 100644
--- a/ell/main.c
+++ b/ell/main.c
@@ -56,7 +56,6 @@ static int epoll_fd;
static bool epoll_running;
static bool epoll_terminate;
static int idle_id;
-static int exit_status = EXIT_FAILURE;
static struct l_queue *idle_list;
@@ -86,9 +85,6 @@ static inline bool __attribute__ ((always_inline)) create_epoll(void)
{
unsigned int i;
- if (likely(epoll_fd))
- return true;
-
epoll_fd = epoll_create1(EPOLL_CLOEXEC);
if (epoll_fd < 0) {
epoll_fd = 0;
@@ -127,7 +123,7 @@ int watch_add(int fd, uint32_t events, watch_event_cb_t callback,
if (unlikely(fd < 0 || !callback))
return -EINVAL;
- if (!create_epoll())
+ if (!epoll_fd)
return -EIO;
if ((unsigned int) fd > watch_entries - 1)
@@ -260,7 +256,7 @@ int idle_add(idle_event_cb_t callback, void *user_data,
if (unlikely(!callback))
return -EINVAL;
- if (!create_epoll())
+ if (!epoll_fd)
return -EIO;
data = l_new(struct idle_data, 1);
@@ -313,25 +309,49 @@ static void idle_dispatch(void *data, void *user_data)
}
/**
+ * l_main_init:
+ *
+ * Initialize the main loop. This must be called before l_main_run()
+ * and any other function that directly or indirectly sets up an idle
+ * or watch. A safe rule-of-thumb is to call it before any function
+ * prefixed with "l_".
+ *
+ * Returns: true if initialization was successful, false otherwise.
+ **/
+LIB_EXPORT bool l_main_init(void)
+{
+ if (unlikely(epoll_running))
+ return false;
+
+ if (!create_epoll())
+ return false;
+
+ epoll_terminate = false;
+
+ return true;
+}
+
+/**
* l_main_run:
*
* Run the main loop
*
- * Returns: #EXIT_SCUCESS after successful execution or #EXIT_FAILURE in
+ * The loop may be restarted by invoking this function after a
+ * previous invocation returns, provided that l_main_exit() has not
+ * been called first.
+ *
+ * Returns: #EXIT_SUCCESS after successful execution or #EXIT_FAILURE in
* case of failure
**/
LIB_EXPORT int l_main_run(void)
{
- unsigned int i;
-
- if (unlikely(epoll_running))
+ /* Has l_main_init() been called? */
+ if (unlikely(!epoll_fd))
return EXIT_FAILURE;
- if (!create_epoll())
+ if (unlikely(epoll_running))
return EXIT_FAILURE;
- epoll_terminate = false;
-
epoll_running = true;
for (;;) {
@@ -375,6 +395,26 @@ LIB_EXPORT int l_main_run(void)
l_queue_foreach_remove(idle_list, idle_prune, NULL);
}
+ epoll_running = false;
+
+ return EXIT_SUCCESS;
+}
+
+/**
+ * l_main_exit:
+ *
+ * Clean up after main loop completes.
+ *
+ **/
+LIB_EXPORT bool l_main_exit(void)
+{
+ unsigned int i;
+
+ if (epoll_running) {
+ l_error("Cleanup attempted on running main loop");
+ return false;
+ }
+
for (i = 0; i < watch_entries; i++) {
struct watch_data *data = watch_list[i];
@@ -399,12 +439,10 @@ LIB_EXPORT int l_main_run(void)
l_queue_destroy(idle_list, idle_destroy);
idle_list = NULL;
- epoll_running = false;
-
close(epoll_fd);
epoll_fd = 0;
- return exit_status;
+ return true;
}
/**
@@ -419,7 +457,6 @@ LIB_EXPORT bool l_main_quit(void)
if (unlikely(!epoll_running))
return false;
- exit_status = EXIT_SUCCESS;
epoll_terminate = true;
return true;
@@ -472,19 +509,3 @@ int l_main_run_with_signal(l_main_signal_cb_t callback, void *user_data)
return result;
}
-
-/**
- * l_main_exit:
- *
- * Teminate the running main loop with exit status
- *
- **/
-LIB_EXPORT
-void l_main_exit(int status)
-{
- if (unlikely(!epoll_running))
- return;
-
- exit_status = status;
- epoll_terminate = true;
-}
diff --git a/ell/main.h b/ell/main.h
index 46d2a13..c23ec55 100644
--- a/ell/main.h
+++ b/ell/main.h
@@ -29,13 +29,15 @@
extern "C" {
#endif
+bool l_main_init(void);
int l_main_run(void);
+bool l_main_exit(void);
+
bool l_main_quit(void);
typedef void (*l_main_signal_cb_t) (uint32_t signo, void *user_data);
int l_main_run_with_signal(l_main_signal_cb_t callback, void *user_data);
-void l_main_exit(int status);
#ifdef __cplusplus
}
--
2.8.4
6 years, 2 months
[PATCH 1/3] main: Manually initialize and clean up the main loop
by Mat Martineau
Automatic initialization and cleanup of the main loop created some
issues with orderly shutdown of an ELL program. If cleanup functions
were called after the l_main_run() returned, they could indirectly set
up the epoll file descriptor and associated resources again. Those
resources were not freed after the cleanup functions ran.
Now, epoll resources are set up by l_main_init() and cleaned up by
l_main_exit(). If cleanup functions set up any idles or watches
(including watch-based timeouts, signals, or ios) after l_main_run()
returns, those will be destroyed by l_main_exit().
l_main_init() must be called before l_main_run() or any other ELL
function that expects epoll to be set up. l_main_exit() should be
called after l_main_run() returns to ensure that all resources are
freed.
---
ell/main.c | 81 +++++++++++++++++++++++++++-----------------
ell/main.h | 4 ++-
examples/dbus-client.c | 5 +++
examples/dbus-service.c | 5 +++
examples/https-client-test.c | 5 +++
examples/https-server-test.c | 5 +++
unit/test-dbus-message-fds.c | 5 +++
unit/test-dbus-properties.c | 5 +++
unit/test-dbus.c | 5 +++
unit/test-genl.c | 5 +++
unit/test-io.c | 5 +++
unit/test-kdbus.c | 5 +++
unit/test-main.c | 5 +++
unit/test-netlink.c | 5 +++
14 files changed, 113 insertions(+), 32 deletions(-)
diff --git a/ell/main.c b/ell/main.c
index f53ed9e..b4c1582 100644
--- a/ell/main.c
+++ b/ell/main.c
@@ -86,9 +86,6 @@ static inline bool __attribute__ ((always_inline)) create_epoll(void)
{
unsigned int i;
- if (likely(epoll_fd))
- return true;
-
epoll_fd = epoll_create1(EPOLL_CLOEXEC);
if (epoll_fd < 0) {
epoll_fd = 0;
@@ -127,7 +124,7 @@ int watch_add(int fd, uint32_t events, watch_event_cb_t callback,
if (unlikely(fd < 0 || !callback))
return -EINVAL;
- if (!create_epoll())
+ if (!epoll_fd)
return -EIO;
if ((unsigned int) fd > watch_entries - 1)
@@ -260,7 +257,7 @@ int idle_add(idle_event_cb_t callback, void *user_data,
if (unlikely(!callback))
return -EINVAL;
- if (!create_epoll())
+ if (!epoll_fd)
return -EIO;
data = l_new(struct idle_data, 1);
@@ -313,25 +310,45 @@ static void idle_dispatch(void *data, void *user_data)
}
/**
+ * l_main_init:
+ *
+ * Initialize the main loop. This must be called before l_main_run()
+ * and any other function that directly or indirectly sets up an idle
+ * or watch. A safe rule-of-thumb is to call it before any function
+ * prefixed with "l_".
+ *
+ * Returns: true if initialization was successful, false otherwise.
+ **/
+LIB_EXPORT bool l_main_init(void)
+{
+ if (unlikely(epoll_running))
+ return false;
+
+ if (!create_epoll())
+ return false;
+
+ epoll_terminate = false;
+
+ return true;
+}
+
+/**
* l_main_run:
*
* Run the main loop
*
- * Returns: #EXIT_SCUCESS after successful execution or #EXIT_FAILURE in
+ * Returns: #EXIT_SUCCESS after successful execution or #EXIT_FAILURE in
* case of failure
**/
LIB_EXPORT int l_main_run(void)
{
- unsigned int i;
-
- if (unlikely(epoll_running))
+ /* Has l_main_init() been called? */
+ if (unlikely(!epoll_fd))
return EXIT_FAILURE;
- if (!create_epoll())
+ if (unlikely(epoll_running))
return EXIT_FAILURE;
- epoll_terminate = false;
-
epoll_running = true;
for (;;) {
@@ -375,6 +392,26 @@ LIB_EXPORT int l_main_run(void)
l_queue_foreach_remove(idle_list, idle_prune, NULL);
}
+ epoll_running = false;
+
+ return exit_status;
+}
+
+/**
+ * l_main_exit:
+ *
+ * Clean up after main loop completes.
+ *
+ **/
+LIB_EXPORT bool l_main_exit(void)
+{
+ unsigned int i;
+
+ if (epoll_running) {
+ l_error("Cleanup attempted on running main loop");
+ return false;
+ }
+
for (i = 0; i < watch_entries; i++) {
struct watch_data *data = watch_list[i];
@@ -399,12 +436,10 @@ LIB_EXPORT int l_main_run(void)
l_queue_destroy(idle_list, idle_destroy);
idle_list = NULL;
- epoll_running = false;
-
close(epoll_fd);
epoll_fd = 0;
- return exit_status;
+ return true;
}
/**
@@ -472,19 +507,3 @@ int l_main_run_with_signal(l_main_signal_cb_t callback, void *user_data)
return result;
}
-
-/**
- * l_main_exit:
- *
- * Teminate the running main loop with exit status
- *
- **/
-LIB_EXPORT
-void l_main_exit(int status)
-{
- if (unlikely(!epoll_running))
- return;
-
- exit_status = status;
- epoll_terminate = true;
-}
diff --git a/ell/main.h b/ell/main.h
index 46d2a13..c23ec55 100644
--- a/ell/main.h
+++ b/ell/main.h
@@ -29,13 +29,15 @@
extern "C" {
#endif
+bool l_main_init(void);
int l_main_run(void);
+bool l_main_exit(void);
+
bool l_main_quit(void);
typedef void (*l_main_signal_cb_t) (uint32_t signo, void *user_data);
int l_main_run_with_signal(l_main_signal_cb_t callback, void *user_data);
-void l_main_exit(int status);
#ifdef __cplusplus
}
diff --git a/examples/dbus-client.c b/examples/dbus-client.c
index be640b7..507a4e8 100644
--- a/examples/dbus-client.c
+++ b/examples/dbus-client.c
@@ -78,6 +78,9 @@ int main(int argc, char *argv[])
sigset_t mask;
uint32_t watch_id;
+ if (!l_main_init())
+ return -1;
+
sigemptyset(&mask);
sigaddset(&mask, SIGINT);
sigaddset(&mask, SIGTERM);
@@ -102,5 +105,7 @@ int main(int argc, char *argv[])
l_dbus_destroy(dbus);
l_signal_remove(signal);
+ l_main_exit();
+
return 0;
}
diff --git a/examples/dbus-service.c b/examples/dbus-service.c
index 51b7e4f..19b17a0 100644
--- a/examples/dbus-service.c
+++ b/examples/dbus-service.c
@@ -181,6 +181,9 @@ int main(int argc, char *argv[])
sigset_t mask;
struct test_data *test;
+ if (!l_main_init())
+ return -1;
+
sigemptyset(&mask);
sigaddset(&mask, SIGINT);
sigaddset(&mask, SIGTERM);
@@ -233,5 +236,7 @@ cleanup:
l_dbus_destroy(dbus);
l_signal_remove(signal);
+ l_main_exit();
+
return 0;
}
diff --git a/examples/https-client-test.c b/examples/https-client-test.c
index 3382970..04a3b37 100644
--- a/examples/https-client-test.c
+++ b/examples/https-client-test.c
@@ -159,6 +159,9 @@ int main(int argc, char *argv[])
return -1;
}
+ if (!l_main_init())
+ return -1;
+
io = l_io_new(fd);
l_io_set_close_on_destroy(io, true);
l_io_set_read_handler(io, https_io_read, tls, NULL);
@@ -176,5 +179,7 @@ int main(int argc, char *argv[])
l_io_destroy(io);
l_tls_free(tls);
+ l_main_exit();
+
return 0;
}
diff --git a/examples/https-server-test.c b/examples/https-server-test.c
index 22a9db1..0f16ed2 100644
--- a/examples/https-server-test.c
+++ b/examples/https-server-test.c
@@ -149,6 +149,9 @@ int main(int argc, char *argv[])
return -1;
}
+ if (!l_main_init())
+ return -1;
+
io = l_io_new(fd);
l_io_set_close_on_destroy(io, true);
l_io_set_read_handler(io, https_io_read, tls, NULL);
@@ -164,5 +167,7 @@ int main(int argc, char *argv[])
l_io_destroy(io);
l_tls_free(tls);
+ l_main_exit();
+
return 0;
}
diff --git a/unit/test-dbus-message-fds.c b/unit/test-dbus-message-fds.c
index 7e710cf..c93f093 100644
--- a/unit/test-dbus-message-fds.c
+++ b/unit/test-dbus-message-fds.c
@@ -330,6 +330,9 @@ int main(int argc, char *argv[])
sigset_t mask;
int i;
+ if (!l_main_init())
+ return -1;
+
test_add("FD passing 1", test_fd_passing_1, NULL);
sigemptyset(&mask);
@@ -377,6 +380,8 @@ done:
l_queue_destroy(tests, l_free);
+ l_main_exit();
+
if (!success)
abort();
diff --git a/unit/test-dbus-properties.c b/unit/test-dbus-properties.c
index 6eae592..b11d1d5 100644
--- a/unit/test-dbus-properties.c
+++ b/unit/test-dbus-properties.c
@@ -978,6 +978,9 @@ int main(int argc, char *argv[])
sigset_t mask;
int i;
+ if (!l_main_init())
+ return -1;
+
test_add("Legacy properties get", test_old_get, NULL);
test_add("Legacy properties set", test_old_set, NULL);
test_add("Legacy optional property", test_old_optional_get, NULL);
@@ -1034,6 +1037,8 @@ done:
l_queue_destroy(tests, l_free);
+ l_main_exit();
+
if (!success)
abort();
diff --git a/unit/test-dbus.c b/unit/test-dbus.c
index b01a530..0fed605 100644
--- a/unit/test-dbus.c
+++ b/unit/test-dbus.c
@@ -201,6 +201,9 @@ int main(int argc, char *argv[])
sigset_t mask;
int i;
+ if (!l_main_init())
+ return -1;
+
sigemptyset(&mask);
sigaddset(&mask, SIGINT);
sigaddset(&mask, SIGTERM);
@@ -248,5 +251,7 @@ int main(int argc, char *argv[])
l_signal_remove(signal);
+ l_main_exit();
+
return 0;
}
diff --git a/unit/test-genl.c b/unit/test-genl.c
index 1764133..d15d92c 100644
--- a/unit/test-genl.c
+++ b/unit/test-genl.c
@@ -44,6 +44,9 @@ int main(int argc, char *argv[])
{
struct l_genl *genl;
+ if (!l_main_init())
+ return -1;
+
l_log_set_stderr();
genl = l_genl_new_default();
@@ -56,5 +59,7 @@ int main(int argc, char *argv[])
l_genl_unref(genl);
+ l_main_exit();
+
return 0;
}
diff --git a/unit/test-io.c b/unit/test-io.c
index 24e1681..922ecad 100644
--- a/unit/test-io.c
+++ b/unit/test-io.c
@@ -75,6 +75,9 @@ int main(int argc, char *argv[])
struct l_io *io1, *io2;
int fd[2];
+ if (!l_main_init())
+ return -1;
+
l_log_set_stderr();
if (socketpair(PF_UNIX, SOCK_STREAM | SOCK_NONBLOCK, 0, fd) < 0) {
@@ -99,5 +102,7 @@ int main(int argc, char *argv[])
l_io_destroy(io2);
l_io_destroy(io1);
+ l_main_exit();
+
return 0;
}
diff --git a/unit/test-kdbus.c b/unit/test-kdbus.c
index 8cac761..ad4a754 100644
--- a/unit/test-kdbus.c
+++ b/unit/test-kdbus.c
@@ -136,6 +136,9 @@ int main(int argc, char *argv[])
struct l_signal *signal;
sigset_t mask;
+ if (!l_main_init())
+ return -1;
+
sigemptyset(&mask);
sigaddset(&mask, SIGINT);
sigaddset(&mask, SIGTERM);
@@ -191,5 +194,7 @@ error:
l_signal_remove(signal);
+ l_main_exit();
+
return EXIT_SUCCESS;
}
diff --git a/unit/test-main.c b/unit/test-main.c
index ee99a4b..1f4bb31 100644
--- a/unit/test-main.c
+++ b/unit/test-main.c
@@ -91,6 +91,9 @@ int main(int argc, char *argv[])
struct l_idle *idle;
sigset_t mask;
+ if (!l_main_init())
+ return -1;
+
sigemptyset(&mask);
sigaddset(&mask, SIGINT);
sigaddset(&mask, SIGTERM);
@@ -129,5 +132,7 @@ int main(int argc, char *argv[])
l_idle_remove(idle);
+ l_main_exit();
+
return 0;
}
diff --git a/unit/test-netlink.c b/unit/test-netlink.c
index a56e5a1..ce522a6 100644
--- a/unit/test-netlink.c
+++ b/unit/test-netlink.c
@@ -77,6 +77,9 @@ int main(int argc, char *argv[])
struct l_netlink *netlink;
struct ifinfomsg msg;
+ if (!l_main_init())
+ return -1;
+
l_log_set_stderr();
netlink = l_netlink_new(NETLINK_ROUTE);
@@ -92,5 +95,7 @@ int main(int argc, char *argv[])
l_netlink_destroy(netlink);
+ l_main_exit();
+
return 0;
}
--
2.8.4
6 years, 2 months
cipher, tls, and keys
by Mat Martineau
So far I've updated the asymmetric cipher code to work with the current
version of the AF_ALG akcipher interface. This leaves the asymmetric
cipher & tls disconnected from the new key/keyring code, which doesn't
take advantage of the kernel's capabilities with akcipher and the keyring.
We have three options:
1. The current code works, so leave it as-is. I took this approach first
to minimize changes while I got it working.
2. Make use of the keyctl() crypto API that's under review. This has the
clearest upstream path because it was implemented by the keyring
maintainer. Would simplify l_asymmetric_cipher code and involve fewer
syscalls.
3. Continue with AF_ALG but make use of ALG_SET_KEY_ID and
ALG_SET_PUB_KEY_ID to use keys already in a keyring. Would not be a big
change, but the kernel patch set is a work in progress and much more
uncertainty about upstream prospects / timing.
I like #2 because it makes for a clean API and simple userspace
implementation. Anyone prefer other options?
--
Mat Martineau
Intel OTC
6 years, 2 months
ell/main.c Memory leak
by Denis Kenzior
This is from iwd:
[DBUS] > 6c 04 01 01 98 00 00 00 04 00 00 00 68 00 00 00 l...........h...
[DBUS] 01 01 6f 00 01 00 00 00 2f 00 00 00 00 00 00 00 ..o...../.......
[DBUS] 03 01 73 00 0f 00 00 00 49 6e 74 65 72 66 61 63 ..s.....Interfac
[DBUS] 65 73 41 64 64 65 64 00 02 01 73 00 22 00 00 00 esAdded...s."...
[DBUS] 6f 72 67 2e 66 72 65 65 64 65 73 6b 74 6f 70 2e org.freedesktop.
[DBUS] 44 42 75 73 2e 4f 62 6a 65 63 74 4d 61 6e 61 67 DBus.ObjectManag
[DBUS] 65 72 00 00 00 00 00 00 08 01 67 00 0a 6f 61 7b er........g..oa{
[DBUS] 73 61 7b 73 76 7d 7d 00 sa{sv}}.
[DBUS] 02 00 00 00 2f 33 00 00 88 00 00 00 00 00 00 00 ..../3..........
[DBUS] 16 00 00 00 6e 65 74 2e 63 6f 6e 6e 6d 61 6e 2e ....net.connman.
[DBUS] 69 77 64 2e 44 65 76 69 63 65 00 00 33 00 00 00 iwd.Device..3...
[DBUS] 04 00 00 00 4e 61 6d 65 00 01 73 00 06 00 00 00 ....Name..s.....
[DBUS] 77 6c 70 32 73 30 00 00 07 00 00 00 41 64 64 72 wlp2s0......Addr
[DBUS] 65 73 73 00 01 73 00 00 06 00 00 00 a0 a8 cd 1c ess..s..........
[DBUS] 7e c9 00 00 00 00 00 00 27 00 00 00 6e 65 74 2e ~.......'...net.
[DBUS] 63 6f 6e 6e 6d 61 6e 2e 69 77 64 2e 57 69 46 69 connman.iwd.WiFi
[DBUS] 53 69 6d 70 6c 65 43 6f 6e 66 69 67 75 72 61 74 SimpleConfigurat
[DBUS] 69 6f 6e 00 00 00 00 00 ion.....
src/scan.c:scan_notify() Scan notification 33
[DBUS] disconnect
D-Bus disconnected, quitting...
src/eap.c:__eap_method_disable()
src/eap.c:eap_md5_exit()
src/eap-tls.c:eap_tls_exit()
src/eap-ttls.c:eap_ttls_exit()
src/main.c:nl80211_vanished() Lost nl80211 interface
src/wsc.c:wsc_exit()
src/scan.c:scan_exit()
src/scan.c:scan_context_free() sc: 0x53f4690
src/netdev.c:netdev_free() Freeing netdev wlp2s0[3]
src/netdev.c:netdev_exit() Closing route netlink socket
src/wiphy.c:wiphy_free() Freeing wiphy phy0
src/dbus.c:dbus_exit()
==11733==
==11733== HEAP SUMMARY:
==11733== in use at exit: 1,048 bytes in 2 blocks
==11733== total heap usage: 759 allocs, 757 frees, 88,760 bytes allocated
==11733==
==11733== 24 bytes in 1 blocks are still reachable in loss record 1 of 2
==11733== at 0x4C2C970: malloc (vg_replace_malloc.c:296)
==11733== by 0x41CA37: l_malloc (util.c:62)
==11733== by 0x41DE00: l_queue_new (queue.c:63)
==11733== by 0x422B9D: create_epoll (main.c:102)
==11733== by 0x422B9D: idle_add (main.c:263)
==11733== by 0x42361A: l_idle_create (idle.c:108)
==11733== by 0x4397AF: schedule_emit_signals (dbus-service.c:1081)
==11733== by 0x43A6EE: _dbus_object_tree_remove_interface
(dbus-service.c:1503)
==11733== by 0x439F45: check_interface_used (dbus-service.c:1306)
==11733== by 0x41F342: l_hashmap_foreach (hashmap.c:532)
==11733== by 0x439FBC: _dbus_object_tree_unregister_interface
(dbus-service.c:1320)
==11733== by 0x42CC6B: l_dbus_unregister_interface (dbus.c:1715)
==11733== by 0x418BE9: wsc_exit (wsc.c:341)
==11733==
==11733== 1,024 bytes in 1 blocks are still reachable in loss record 2 of 2
==11733== at 0x4C2C970: malloc (vg_replace_malloc.c:296)
==11733== by 0x422B85: create_epoll (main.c:98)
==11733== by 0x422B85: idle_add (main.c:263)
==11733== by 0x42361A: l_idle_create (idle.c:108)
==11733== by 0x4397AF: schedule_emit_signals (dbus-service.c:1081)
==11733== by 0x43A6EE: _dbus_object_tree_remove_interface
(dbus-service.c:1503)
==11733== by 0x439F45: check_interface_used (dbus-service.c:1306)
==11733== by 0x41F342: l_hashmap_foreach (hashmap.c:532)
==11733== by 0x439FBC: _dbus_object_tree_unregister_interface
(dbus-service.c:1320)
==11733== by 0x42CC6B: l_dbus_unregister_interface (dbus.c:1715)
==11733== by 0x418BE9: wsc_exit (wsc.c:341)
==11733== by 0x4029D3: nl80211_vanished (main.c:117)
==11733== by 0x428D49: l_genl_family_unref (genl.c:1057)
==11733==
==11733== LEAK SUMMARY:
==11733== definitely lost: 0 bytes in 0 blocks
==11733== indirectly lost: 0 bytes in 0 blocks
==11733== possibly lost: 0 bytes in 0 blocks
==11733== still reachable: 1,048 bytes in 2 blocks
==11733== suppressed: 0 bytes in 0 blocks
This leak happens in create_epoll(). The sequence of events is a bit
bizarre:
1. We send an invalid signal
2. DBus detects that and kills our connection
3. l_main_quit is called, which frees all main event loop resources
4. We start tearing down the rest of the daemon
a) At some point we unregister some interfaces
b) ObjectManager schedules the transmission of a signal
c) idle_create gets called, which calls create_epoll()
5. DBus is destroyed, which removes the idle source in 4c
Unfortunately at this point, nothing actually frees the resources
created by create_epoll().
I'm thinking l_main_run needs to be split up into l_main_new, l_main_run
and l_main_destroy.
Thoughts?
Regards,
-Denis
6 years, 2 months