From 5599e74b1d9e2b3e3bbd87af494b1de487d8ec21 Mon Sep 17 00:00:00 2001 From: Benjamin Marzinski Date: Fri, 30 Apr 2021 14:11:17 -0500 Subject: [PATCH 1/8] resource: ubridge: dump udev namespace keys The arguments to _build_kv_buffer() have been changed to make it possible to control whether or not udev keys are dumped. _cmd_exec_dump() now also dumps the reserved udev keys that are saved to the main kv_store. --- src/resource/ubridge.c | 36 +++++++++++++++++++----------------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/src/resource/ubridge.c b/src/resource/ubridge.c index 76d6614c..3574aa43 100644 --- a/src/resource/ubridge.c +++ b/src/resource/ubridge.c @@ -881,7 +881,7 @@ int sid_ucmd_print_exported_kv_store(const char *prefix, char *ptr, size_t size, return 0; } -static int _build_kv_buffer(sid_resource_t *cmd_res, struct buffer **buf, bool export_udev, bool export_sid, bool persistent_only) +static int _build_kv_buffer(sid_resource_t *cmd_res, struct buffer **buf, bool export_udev, bool export_sid, bool is_dump) { struct sid_ucmd_ctx * ucmd_ctx = sid_resource_get_data(cmd_res); struct kv_value * kv_value; @@ -930,7 +930,7 @@ static int _build_kv_buffer(sid_resource_t *cmd_res, struct buffer **buf, bool e iov_size = size; kv_value = NULL; - if (persistent_only) { + if (!is_dump) { if (!(KV_VALUE_FLAGS(iov) & KV_PERSISTENT)) continue; @@ -941,7 +941,7 @@ static int _build_kv_buffer(sid_resource_t *cmd_res, struct buffer **buf, bool e iov_size = 0; kv_value = value; - if (persistent_only) { + if (!is_dump) { if (!(kv_value->flags & KV_PERSISTENT)) continue; @@ -967,18 +967,20 @@ static int _build_kv_buffer(sid_resource_t *cmd_res, struct buffer **buf, bool e r = -ENOTSUP; goto fail; } - key = _get_key_part(key, KEY_PART_CORE, NULL); - if (!buffer_add(ucmd_ctx->res_buf, (void *) key, strlen(key), &r) || - !buffer_add(ucmd_ctx->res_buf, KV_PAIR_C, 1, &r)) - goto fail; - data_offset = _kv_value_ext_data_offset(kv_value); - if (!buffer_add(ucmd_ctx->res_buf, - kv_value->data + data_offset, - strlen(kv_value->data + data_offset), - &r) || - !buffer_add(ucmd_ctx->res_buf, KV_END_C, 1, &r)) - goto fail; - continue; + if (!is_dump) { + key = _get_key_part(key, KEY_PART_CORE, NULL); + if (!buffer_add(ucmd_ctx->res_buf, (void *) key, strlen(key), &r) || + !buffer_add(ucmd_ctx->res_buf, KV_PAIR_C, 1, &r)) + goto fail; + data_offset = _kv_value_ext_data_offset(kv_value); + if (!buffer_add(ucmd_ctx->res_buf, + kv_value->data + data_offset, + strlen(kv_value->data + data_offset), + &r) || + !buffer_add(ucmd_ctx->res_buf, KV_END_C, 1, &r)) + goto fail; + continue; + } } /* @@ -2003,7 +2005,7 @@ static int _cmd_exec_dump(struct cmd_exec_arg *exec_arg) buffer_get_data(ucmd_ctx->res_buf, (const void **) &response_header, &size); - if ((r = _build_kv_buffer(exec_arg->cmd_res, &export_buf, false, true, false)) < 0) + if ((r = _build_kv_buffer(exec_arg->cmd_res, &export_buf, true, true, true)) < 0) response_header->status |= COMMAND_STATUS_FAILURE; if (buffer_write_all(ucmd_ctx->res_buf, conn->fd) < 0) { @@ -3517,7 +3519,7 @@ static int _export_kv_store(sid_resource_t *cmd_res) &export_buf, _cmd_regs[ucmd_ctx->request_header.cmd].flags & CMD_KV_EXPORT_UDEV, _cmd_regs[ucmd_ctx->request_header.cmd].flags & CMD_KV_EXPORT_SID, - true)) < 0) + false)) < 0) return r; data_spec.data = NULL; From 7d24c5e30108708fb80d595488fc40086a751686 Mon Sep 17 00:00:00 2001 From: Benjamin Marzinski Date: Mon, 3 May 2021 11:54:45 -0500 Subject: [PATCH 2/8] buffer: allow adding NULL data to linear buffers With vector buffers, it is possible to first allocate space and add it to the buffer, and then fill it. This wasn't possible with linear buffers, which required already existing space to copy the data from. buffer_add() should allow a NULL data pointer, and simply memset the allocated space to zero in this case. This allows linear buffers to be used with functions that are designed to fill preallocated memory. --- src/base/buffer-type-linear.c | 5 ++++- src/base/buffer-type-vector.c | 4 ++-- src/base/buffer.c | 6 ------ 3 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/base/buffer-type-linear.c b/src/base/buffer-type-linear.c index bb7961f5..b52926d1 100644 --- a/src/base/buffer-type-linear.c +++ b/src/base/buffer-type-linear.c @@ -154,7 +154,10 @@ static const void *_buffer_linear_add(struct buffer *buf, void *data, size_t len goto out; start = buf->mem + used; - memcpy(start, data, len); + if (data) + memcpy(start, data, len); + else + memset(start, 0, len); buf->stat.usage.used = used + len; out: if (ret_code) diff --git a/src/base/buffer-type-vector.c b/src/base/buffer-type-vector.c index 972a1db5..f0a93a89 100644 --- a/src/base/buffer-type-vector.c +++ b/src/base/buffer-type-vector.c @@ -184,9 +184,9 @@ const void *_buffer_vector_add(struct buffer *buf, void *data, size_t len, int * struct iovec *iov; int r; - if (buf == NULL || buf->mem == NULL) { + if (buf == NULL || buf->mem == NULL || data == NULL) { if (ret_code) - *ret_code = EINVAL; + *ret_code = -EINVAL; return NULL; } diff --git a/src/base/buffer.c b/src/base/buffer.c index 75055271..e1ed6105 100644 --- a/src/base/buffer.c +++ b/src/base/buffer.c @@ -101,12 +101,6 @@ int buffer_reset(struct buffer *buf) const void *buffer_add(struct buffer *buf, void *data, size_t len, int *ret_code) { - if (!data) { - if (*ret_code) - *ret_code = -EINVAL; - return NULL; - } - return _buffer_type_registry[buf->stat.spec.type]->add(buf, data, len, ret_code); } From 6f167c647d07643ee41bac35ac9554e3bdbb3589 Mon Sep 17 00:00:00 2001 From: Benjamin Marzinski Date: Mon, 3 May 2021 14:45:52 -0500 Subject: [PATCH 3/8] base: add base64 encoding code. Copy BSD Licensed base64 code from: https://web.mit.edu/freebsd/head/contrib/wpa/src/utils/base64.c https://web.mit.edu/freebsd/head/contrib/wpa/src/utils/base64.h The source code has had its includes updated to compile correctly in the sid code base, and its LICENSE reference comments updated to point to the LICENSE file in the sid code. It has also been reformatted with to match the sid formatting style. and BSD License from: https://web.mit.edu/freebsd/head/contrib/wpa/ --- BSD_LICENSE | 29 ++++++++ rpm/sid.spec | 2 +- src/base/Makefile.am | 3 +- src/base/base64.c | 152 ++++++++++++++++++++++++++++++++++++++ src/include/base/base64.h | 18 +++++ 5 files changed, 202 insertions(+), 2 deletions(-) create mode 100644 BSD_LICENSE create mode 100644 src/base/base64.c create mode 100644 src/include/base/base64.h diff --git a/BSD_LICENSE b/BSD_LICENSE new file mode 100644 index 00000000..5a71b118 --- /dev/null +++ b/BSD_LICENSE @@ -0,0 +1,29 @@ +Copyright (c) 2002-2012, Jouni Malinen and contributors +All Rights Reserved. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions are +met: + +1. Redistributions of source code must retain the above copyright + notice, this list of conditions and the following disclaimer. + +2. Redistributions in binary form must reproduce the above copyright + notice, this list of conditions and the following disclaimer in the + documentation and/or other materials provided with the distribution. + +3. Neither the name(s) of the above-listed copyright holder(s) nor the + names of its contributors may be used to endorse or promote products + derived from this software without specific prior written permission. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. diff --git a/rpm/sid.spec b/rpm/sid.spec index 1f4b94de..93f08348 100644 --- a/rpm/sid.spec +++ b/rpm/sid.spec @@ -79,7 +79,7 @@ rm -f %{buildroot}/%{_libdir}/sid/modules/ucmd/type/*.{a,la} %multilib_fix_c_header --file %{_includedir}/sid/config.h %files -%license COPYING +%license COPYING BSD_LICENSE %{_sbindir}/sid %config(noreplace) %{_sysconfdir}/sysconfig/sid.sysconfig %{_udevrulesdir}/00-sid.rules diff --git a/src/base/Makefile.am b/src/base/Makefile.am index 158d8734..004efd90 100644 --- a/src/base/Makefile.am +++ b/src/base/Makefile.am @@ -29,7 +29,8 @@ libsidbase_la_SOURCES = mem.c \ comms.c \ util.c \ hash.c \ - formatter.c + formatter.c \ + base64.c basedir = $(pkgincludedir)/base diff --git a/src/base/base64.c b/src/base/base64.c new file mode 100644 index 00000000..f41241aa --- /dev/null +++ b/src/base/base64.c @@ -0,0 +1,152 @@ +/* + * Base64 encoding/decoding (RFC1341) + * Copyright (c) 2005-2011, Jouni Malinen + * Copyright (c) 2021 Red Hat, Inc. All rights reserved. + * + * This software may be distributed under the terms of the BSD license. + * See BSD_LICENSE for more details. + * SPDX-License-Identifier: BSD-3-Clause + */ + +#include "base/base64.h" + +#include +#include + +static const unsigned char base64_table[65] = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/"; + +/** + * base64_encode - Base64 encode + * @src: Data to be encoded + * @len: Length of the data to be encoded + * @out_len: Pointer to output length variable, or %NULL if not used + * Returns: Allocated buffer of out_len bytes of encoded data, + * or %NULL on failure + * + * Caller is responsible for freeing the returned buffer. Returned buffer is + * nul terminated to make it easier to use as a C string. The nul terminator is + * not included in out_len. + */ +unsigned char *base64_encode(const unsigned char *src, size_t len, size_t *out_len) +{ + unsigned char * out, *pos; + const unsigned char *end, *in; + size_t olen; + int line_len; + + olen = len * 4 / 3 + 4; /* 3-byte blocks to 4-byte */ + olen += olen / 72; /* line feeds */ + olen++; /* nul termination */ + if (olen < len) + return NULL; /* integer overflow */ + out = malloc(olen); + if (out == NULL) + return NULL; + + end = src + len; + in = src; + pos = out; + line_len = 0; + while (end - in >= 3) { + *pos++ = base64_table[in[0] >> 2]; + *pos++ = base64_table[((in[0] & 0x03) << 4) | (in[1] >> 4)]; + *pos++ = base64_table[((in[1] & 0x0f) << 2) | (in[2] >> 6)]; + *pos++ = base64_table[in[2] & 0x3f]; + in += 3; + line_len += 4; + if (line_len >= 72) { + *pos++ = '\n'; + line_len = 0; + } + } + + if (end - in) { + *pos++ = base64_table[in[0] >> 2]; + if (end - in == 1) { + *pos++ = base64_table[(in[0] & 0x03) << 4]; + *pos++ = '='; + } else { + *pos++ = base64_table[((in[0] & 0x03) << 4) | (in[1] >> 4)]; + *pos++ = base64_table[(in[1] & 0x0f) << 2]; + } + *pos++ = '='; + line_len += 4; + } + + if (line_len) + *pos++ = '\n'; + + *pos = '\0'; + if (out_len) + *out_len = pos - out; + return out; +} + +/** + * base64_decode - Base64 decode + * @src: Data to be decoded + * @len: Length of the data to be decoded + * @out_len: Pointer to output length variable + * Returns: Allocated buffer of out_len bytes of decoded data, + * or %NULL on failure + * + * Caller is responsible for freeing the returned buffer. + */ +unsigned char *base64_decode(const unsigned char *src, size_t len, size_t *out_len) +{ + unsigned char dtable[256], *out, *pos, block[4], tmp; + size_t i, count, olen; + int pad = 0; + + memset(dtable, 0x80, 256); + for (i = 0; i < sizeof(base64_table) - 1; i++) + dtable[base64_table[i]] = (unsigned char) i; + dtable['='] = 0; + + count = 0; + for (i = 0; i < len; i++) { + if (dtable[src[i]] != 0x80) + count++; + } + + if (count == 0 || count % 4) + return NULL; + + olen = count / 4 * 3; + pos = out = malloc(olen); + if (out == NULL) + return NULL; + + count = 0; + for (i = 0; i < len; i++) { + tmp = dtable[src[i]]; + if (tmp == 0x80) + continue; + + if (src[i] == '=') + pad++; + block[count] = tmp; + count++; + if (count == 4) { + *pos++ = (block[0] << 2) | (block[1] >> 4); + *pos++ = (block[1] << 4) | (block[2] >> 2); + *pos++ = (block[2] << 6) | block[3]; + count = 0; + if (pad) { + if (pad == 1) + pos--; + else if (pad == 2) + pos -= 2; + else { + /* Invalid padding */ + free(out); + return NULL; + } + break; + } + } + } + + *out_len = pos - out; + return out; +} diff --git a/src/include/base/base64.h b/src/include/base/base64.h new file mode 100644 index 00000000..4b35eeff --- /dev/null +++ b/src/include/base/base64.h @@ -0,0 +1,18 @@ +/* + * Base64 encoding/decoding (RFC1341) + * Copyright (c) 2005, Jouni Malinen + * + * This software may be distributed under the terms of the BSD license. + * See BSD_LICENSE for more details. + * SPDX-License-Identifier: BSD-3-Clause + */ + +#ifndef BASE64_H +#define BASE64_H + +#include + +unsigned char *base64_encode(const unsigned char *src, size_t len, size_t *out_len); +unsigned char *base64_decode(const unsigned char *src, size_t len, size_t *out_len); + +#endif /* BASE64_H */ From a421bce1ea02c43267315ba7b3aa3dde3995e53b Mon Sep 17 00:00:00 2001 From: Benjamin Marzinski Date: Mon, 3 May 2021 17:30:49 -0500 Subject: [PATCH 4/8] formatter: base64 encoded binary print functions Add print_binary_* functions, to print base64 binary data. In order to keep from mallocing twice, add base64_len_encode() to get the encoded string size, and modify base64_encode() to encode the data to a preallocated buffer. --- src/base/base64.c | 72 ++++++++++++++++++------------------ src/base/formatter.c | 48 ++++++++++++++++++++++++ src/include/base/base64.h | 3 +- src/include/base/formatter.h | 8 ++++ 4 files changed, 94 insertions(+), 37 deletions(-) diff --git a/src/base/base64.c b/src/base/base64.c index f41241aa..d912357d 100644 --- a/src/base/base64.c +++ b/src/base/base64.c @@ -10,54 +10,60 @@ #include "base/base64.h" +#include #include #include static const unsigned char base64_table[65] = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/"; +/** + * base64_len_encode - Size necessary for base64_encode + * @in_len: Length of the data to be encoded + * Returns: output length needed to store the base64 encoded data, including + * padding and NULL bytes, or 0 if the buffer overflowed. + */ + +size_t base64_len_encode(size_t in_len) +{ + size_t out_len = 1; /* NULL termination */ + + if (!in_len) + return out_len; + out_len += ((in_len - 1) / 3 + 1) * 4; /* 4 bytes for every 3 (rounded up) */ + if (out_len < in_len) + return 0; + return out_len; +} + /** * base64_encode - Base64 encode * @src: Data to be encoded - * @len: Length of the data to be encoded - * @out_len: Pointer to output length variable, or %NULL if not used - * Returns: Allocated buffer of out_len bytes of encoded data, - * or %NULL on failure + * @in_len: Length of the data to be encoded + * @dest: pre-allocated buffer to store the encoded data + * @out_len: Length of the encoded data + * Returns: buffer of out_len bytes of encoded data * - * Caller is responsible for freeing the returned buffer. Returned buffer is - * nul terminated to make it easier to use as a C string. The nul terminator is - * not included in out_len. + * Returned buffer is nul terminated to make it easier to use as a C string. */ -unsigned char *base64_encode(const unsigned char *src, size_t len, size_t *out_len) +int base64_encode(const unsigned char *src, size_t in_len, unsigned char *dest, size_t out_len) { - unsigned char * out, *pos; + unsigned char * pos; const unsigned char *end, *in; - size_t olen; - int line_len; - - olen = len * 4 / 3 + 4; /* 3-byte blocks to 4-byte */ - olen += olen / 72; /* line feeds */ - olen++; /* nul termination */ - if (olen < len) - return NULL; /* integer overflow */ - out = malloc(olen); - if (out == NULL) - return NULL; + size_t check_size; + + check_size = base64_len_encode(in_len); + if (check_size == 0 || check_size > out_len) + return -EINVAL; - end = src + len; - in = src; - pos = out; - line_len = 0; + end = src + in_len; + in = src; + pos = dest; while (end - in >= 3) { *pos++ = base64_table[in[0] >> 2]; *pos++ = base64_table[((in[0] & 0x03) << 4) | (in[1] >> 4)]; *pos++ = base64_table[((in[1] & 0x0f) << 2) | (in[2] >> 6)]; *pos++ = base64_table[in[2] & 0x3f]; in += 3; - line_len += 4; - if (line_len >= 72) { - *pos++ = '\n'; - line_len = 0; - } } if (end - in) { @@ -70,16 +76,10 @@ unsigned char *base64_encode(const unsigned char *src, size_t len, size_t *out_l *pos++ = base64_table[(in[1] & 0x0f) << 2]; } *pos++ = '='; - line_len += 4; } - if (line_len) - *pos++ = '\n'; - *pos = '\0'; - if (out_len) - *out_len = pos - out; - return out; + return 0; } /** diff --git a/src/base/formatter.c b/src/base/formatter.c index 4b5d9317..7ab33cd2 100644 --- a/src/base/formatter.c +++ b/src/base/formatter.c @@ -19,6 +19,8 @@ */ #include "base/formatter.h" +#include "base/base64.h" + #include #include @@ -40,6 +42,14 @@ static void _print_fmt(struct buffer *buf, const char *fmt, ...) va_end(ap); } +static void _print_binary(unsigned char *value, size_t len, struct buffer *buf) +{ + size_t enc_len = base64_len_encode(len); + const char *ptr = buffer_add(buf, NULL, enc_len, NULL); + base64_encode(value, len, (unsigned char *) ptr, enc_len); + buffer_rewind(buf, 1, BUFFER_POS_REL); +} + void print_indent(int level, struct buffer *buf) { for (int i = 0; i < level; i++) { @@ -123,6 +133,29 @@ void print_str_field(char *field_name, char *value, output_format_t format, stru } } +void print_binary_field(char * field_name, + char * value, + size_t len, + output_format_t format, + struct buffer * buf, + bool trailing_comma, + int level) +{ + if (format == JSON) { + print_indent(level, buf); + _print_fmt(buf, "\"%s\": \"", field_name); + _print_binary((unsigned char *) value, len, buf); + _print_fmt(buf, "\""); + if (trailing_comma) + _print_fmt(buf, ","); + _print_fmt(buf, "\n"); + } else { + _print_fmt(buf, "%s: ", field_name); + _print_binary((unsigned char *) value, len, buf); + _print_fmt(buf, "\n"); + } +} + void print_uint_field(char *field_name, uint value, output_format_t format, struct buffer *buf, bool trailing_comma, int level) { if (format == JSON) { @@ -219,6 +252,21 @@ void print_str_array_elem(char *value, output_format_t format, struct buffer *bu } } +void print_binary_array_elem(char *value, size_t len, output_format_t format, struct buffer *buf, bool trailing_comma, int level) +{ + if (format == JSON) { + print_indent(level, buf); + _print_fmt(buf, "\""); + _print_binary((unsigned char *) value, len, buf); + _print_fmt(buf, "\""); + if (trailing_comma) + _print_fmt(buf, ",\n"); + } else { + _print_binary((unsigned char *) value, len, buf); + _print_fmt(buf, "\n"); + } +} + void print_null_byte(struct buffer *buf) { buffer_fmt_add(buf, NULL, ""); diff --git a/src/include/base/base64.h b/src/include/base/base64.h index 4b35eeff..bc39d955 100644 --- a/src/include/base/base64.h +++ b/src/include/base/base64.h @@ -12,7 +12,8 @@ #include -unsigned char *base64_encode(const unsigned char *src, size_t len, size_t *out_len); +size_t base64_len_encode(size_t in_len); +int base64_encode(const unsigned char *src, size_t len, unsigned char *dest, size_t out_len); unsigned char *base64_decode(const unsigned char *src, size_t len, size_t *out_len); #endif /* BASE64_H */ diff --git a/src/include/base/formatter.h b/src/include/base/formatter.h index fb1316aa..a75d7608 100644 --- a/src/include/base/formatter.h +++ b/src/include/base/formatter.h @@ -45,6 +45,13 @@ void print_end_array(bool needs_comma, output_format_t format, struct buffer *bu void print_start_elem(bool needs_comma, output_format_t format, struct buffer *buf, int level); void print_end_elem(output_format_t format, struct buffer *buf, int level); void print_str_field(char *field_name, char *value, output_format_t format, struct buffer *buf, bool trailing_comma, int level); +void print_binary_field(char * field_name, + char * value, + size_t len, + output_format_t format, + struct buffer * buf, + bool trailing_comma, + int level); void print_uint_field(char *field_name, uint value, output_format_t format, struct buffer *buf, bool trailing_comma, int level); void print_uint64_field(char * field_name, uint64_t value, @@ -66,6 +73,7 @@ void print_bool_array_elem(char * field_name, int level); void print_uint_array_elem(uint value, output_format_t format, struct buffer *buf, bool trailing_comma, int level); void print_str_array_elem(char *value, output_format_t format, struct buffer *buf, bool trailing_comma, int level); +void print_binary_array_elem(char *value, size_t len, output_format_t format, struct buffer *buf, bool trailing_comma, int level); void print_null_byte(struct buffer *buf); #ifdef __cplusplus From 3a325f1bf80591be622387d694943b5579bb458a Mon Sep 17 00:00:00 2001 From: Benjamin Marzinski Date: Mon, 3 May 2021 17:56:16 -0500 Subject: [PATCH 5/8] resource: ubridge: dump binary values with base64 encoding If the value doesn't only include printing characters or doesn't end with a NULL byte, print it as base64 encoded binary data. --- src/resource/ubridge.c | 36 ++++++++++++++++++++++++++++++------ 1 file changed, 30 insertions(+), 6 deletions(-) diff --git a/src/resource/ubridge.c b/src/resource/ubridge.c index 3574aa43..bf191e9e 100644 --- a/src/resource/ubridge.c +++ b/src/resource/ubridge.c @@ -741,6 +741,18 @@ static size_t _kv_value_ext_data_offset(struct kv_value *kv_value) return strlen(kv_value->data) + 1; } +bool _is_string_data(char *ptr, size_t len) +{ + int i; + + if (ptr[len - 1] != '\0') + return false; + for (i = 0; i < len - 1; i++) + if (!isprint(ptr[i])) + return false; + return true; +} + int sid_ucmd_print_exported_kv_store(const char *prefix, char *ptr, size_t size, output_format_t format, struct buffer *outbuf) { size_t full_key_size, data_size, len; @@ -837,9 +849,17 @@ int sid_ucmd_print_exported_kv_store(const char *prefix, char *ptr, size_t size, print_start_array("values", format, outbuf, 3); /* fall through */ default: - if (len) - print_str_array_elem(ptr, format, outbuf, i + 1 < data_size, 4); - else + if (len) { + if (_is_string_data(ptr, len)) + print_str_array_elem(ptr, format, outbuf, i + 1 < data_size, 4); + else + print_binary_array_elem(ptr, + len, + format, + outbuf, + i + 1 < data_size, + 4); + } else print_str_array_elem("", format, outbuf, i + 1 < data_size, 4); } ptr += len; @@ -865,9 +885,13 @@ int sid_ucmd_print_exported_kv_store(const char *prefix, char *ptr, size_t size, print_str_field("owner", data, format, outbuf, true, 3); owner_size = strlen(data) + 1; data += owner_size; - if (sizeof(value) + owner_size < data_size) - print_str_field("value", data, format, outbuf, true, 3); - else + if (sizeof(value) + owner_size < data_size) { + len = data_size - (sizeof(value) + owner_size); + if (_is_string_data(data, len)) + print_str_field("value", data, format, outbuf, true, 3); + else + print_binary_field("value", data, len, format, outbuf, true, 3); + } else print_str_field("value", "", format, outbuf, true, 3); ptr += data_size; } From bb1928c2406cb4082a67e03d3556da5e90817c17 Mon Sep 17 00:00:00 2001 From: Benjamin Marzinski Date: Wed, 5 May 2021 14:45:36 -0500 Subject: [PATCH 6/8] buffer: Fix empty size-prefixed buffer issues Rewinding an empty size-prefixed buffer failed, even if the rewind target was valid (just after the size prefix). Calling buffer_get_data() on an empty size-prefixed buffer would return a value around SIZE_MAX, due to a size_t rollover. Also, the functions that read from a passed fd assumed that the size_prefix would always be non-zero, since mmap requires a non-zero length. This wasn't true for empty size-prefixed buffers. --- src/base/buffer-type-linear.c | 5 ++++- src/base/buffer-type-vector.c | 5 ++++- src/base/buffer.c | 2 ++ src/resource/ubridge.c | 5 +++++ src/tools/sidctl/sidctl.c | 4 ++++ 5 files changed, 19 insertions(+), 2 deletions(-) diff --git a/src/base/buffer-type-linear.c b/src/base/buffer-type-linear.c index b52926d1..5922585a 100644 --- a/src/base/buffer-type-linear.c +++ b/src/base/buffer-type-linear.c @@ -209,6 +209,9 @@ static int _buffer_linear_rewind(struct buffer *buf, size_t pos) { size_t min_pos = (buf->stat.spec.mode == BUFFER_MODE_SIZE_PREFIX) ? BUFFER_SIZE_PREFIX_LEN : 0; + if (!buf->stat.usage.used && pos == min_pos) + return 0; + if (pos > buf->stat.usage.used || pos < min_pos) return -EINVAL; @@ -262,7 +265,7 @@ static int _buffer_linear_get_data(struct buffer *buf, const void **data, size_t if (data) *data = buf->mem + BUFFER_SIZE_PREFIX_LEN; if (data_size) - *data_size = buf->stat.usage.used - BUFFER_SIZE_PREFIX_LEN; + *data_size = (buf->stat.usage.used) ? buf->stat.usage.used - BUFFER_SIZE_PREFIX_LEN : 0; break; default: return -ENOTSUP; diff --git a/src/base/buffer-type-vector.c b/src/base/buffer-type-vector.c index f0a93a89..7afb92a0 100644 --- a/src/base/buffer-type-vector.c +++ b/src/base/buffer-type-vector.c @@ -221,6 +221,9 @@ int _buffer_vector_rewind(struct buffer *buf, size_t pos) { size_t min_pos = (buf->stat.spec.mode == BUFFER_MODE_SIZE_PREFIX) ? 1 : 0; + if (!buf->stat.usage.used && pos == min_pos) + return 0; + if (pos > buf->stat.usage.used || pos < min_pos) return -EINVAL; @@ -269,7 +272,7 @@ int _buffer_vector_get_data(struct buffer *buf, const void **data, size_t *data_ if (data) *data = buf->mem + VECTOR_ITEM_SIZE; if (data_size) - *data_size = buf->stat.usage.used - 1; + *data_size = (buf->stat.usage.used) ? buf->stat.usage.used - 1 : 0; break; default: return -ENOTSUP; diff --git a/src/base/buffer.c b/src/base/buffer.c index e1ed6105..eb60d0c9 100644 --- a/src/base/buffer.c +++ b/src/base/buffer.c @@ -124,6 +124,8 @@ const void *buffer_vfmt_add(struct buffer *buf, int *ret_code, const char *fmt, int buffer_rewind(struct buffer *buf, size_t pos, buffer_pos_t whence) { if (whence == BUFFER_POS_REL) { + if (pos == 0) + return 0; /* otherwise this fails on an empty size-prefixed buffer */ if (pos > buf->stat.usage.used) return -EINVAL; diff --git a/src/resource/ubridge.c b/src/resource/ubridge.c index bf191e9e..f5ef3f2e 100644 --- a/src/resource/ubridge.c +++ b/src/resource/ubridge.c @@ -3957,6 +3957,11 @@ static int _sync_main_kv_store(sid_resource_t *worker_proxy_res, sid_resource_t goto out; } + if (msg_size <= BUFFER_SIZE_PREFIX_LEN) { /* nothing to sync */ + r = 0; + goto out; + } + if ((p = shm = mmap(NULL, msg_size, PROT_READ, MAP_SHARED, fd, 0)) == MAP_FAILED) { log_error_errno(ID(worker_proxy_res), errno, "Failed to map memory with key-value store"); goto out; diff --git a/src/tools/sidctl/sidctl.c b/src/tools/sidctl/sidctl.c index 9c7f20a7..77861c06 100644 --- a/src/tools/sidctl/sidctl.c +++ b/src/tools/sidctl/sidctl.c @@ -95,6 +95,10 @@ static int _usid_cmd_dump(struct args *args, output_format_t format, struct buff r = -errno; goto out; } + if (msg_size <= BUFFER_SIZE_PREFIX_LEN) { + r = sid_ucmd_print_exported_kv_store(LOG_PREFIX, NULL, 0, format, outbuf); + goto out; + } if ((shm = mmap(NULL, msg_size, PROT_READ, MAP_SHARED, fd, 0)) == MAP_FAILED) { log_error_errno(LOG_PREFIX, errno, "Failed to map memory with key-value store"); r = -errno; From 361ccc341d0cdbd9299003e807a6f374b8401030 Mon Sep 17 00:00:00 2001 From: Benjamin Marzinski Date: Wed, 5 May 2021 15:58:46 -0500 Subject: [PATCH 7/8] ubridge: fixed check for mmaped memfd When mmap() fails, it returns MAP_FAILED, not NULL, so check for that to see if we need to munmap(). --- src/resource/ubridge.c | 4 ++-- src/tools/sidctl/sidctl.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/resource/ubridge.c b/src/resource/ubridge.c index f5ef3f2e..a26abdf3 100644 --- a/src/resource/ubridge.c +++ b/src/resource/ubridge.c @@ -3934,7 +3934,7 @@ static int _sync_main_kv_store(sid_resource_t *worker_proxy_res, sid_resource_t kv_store_value_flags_t flags; BUFFER_SIZE_PREFIX_TYPE msg_size; size_t full_key_size, data_size, data_offset, i; - char * full_key, *shm = NULL, *p, *end; + char * full_key, *shm = MAP_FAILED, *p, *end; struct kv_value * value = NULL; struct iovec * iov = NULL; const char * iov_str; @@ -4092,7 +4092,7 @@ static int _sync_main_kv_store(sid_resource_t *worker_proxy_res, sid_resource_t out: free(iov); - if (shm && munmap(shm, msg_size) < 0) { + if (shm != MAP_FAILED && munmap(shm, msg_size) < 0) { log_error_errno(ID(worker_proxy_res), errno, "Failed to unmap memory with key-value store"); r = -1; } diff --git a/src/tools/sidctl/sidctl.c b/src/tools/sidctl/sidctl.c index 77861c06..3eef4de9 100644 --- a/src/tools/sidctl/sidctl.c +++ b/src/tools/sidctl/sidctl.c @@ -79,7 +79,7 @@ static int _usid_cmd_dump(struct args *args, output_format_t format, struct buff struct usid_msg_header *msg; int r; int fd = -1; - char * shm = NULL; + char * shm = MAP_FAILED; if ((r = usid_req(LOG_PREFIX, USID_CMD_DUMP, 0, NULL, NULL, &readbuf, &fd)) < 0) return r; @@ -111,7 +111,7 @@ static int _usid_cmd_dump(struct args *args, output_format_t format, struct buff format, outbuf); out: - if (shm && munmap(shm, msg_size) < 0) { + if (shm != MAP_FAILED && munmap(shm, msg_size) < 0) { log_error_errno(LOG_PREFIX, errno, "Failed to unmap memory with key-value store"); r = -1; } From fcce25bf978768fdca9aac9d1fe30736d2d64abc Mon Sep 17 00:00:00 2001 From: Benjamin Marzinski Date: Wed, 5 May 2021 16:45:44 -0500 Subject: [PATCH 8/8] ubridge: check size of data from client connection --- src/resource/ubridge.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/resource/ubridge.c b/src/resource/ubridge.c index a26abdf3..ea5cd484 100644 --- a/src/resource/ubridge.c +++ b/src/resource/ubridge.c @@ -3655,6 +3655,10 @@ static int _on_connection_event(sid_resource_event_source_t *es, int fd, uint32_ if (buffer_is_complete(conn->buf, NULL)) { (void) buffer_get_data(conn->buf, (const void **) &msg.header, &msg.size); + if (msg.size < sizeof(struct usid_msg_header)) { + (void) _connection_cleanup(conn_res); + return -1; + } /* Sanitize command number - map all out of range command numbers to CMD_UNKNOWN. */ if (msg.header->cmd < _USID_CMD_START || msg.header->cmd > _USID_CMD_END) msg.header->cmd = USID_CMD_UNKNOWN;