| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <2a2c5072-8cd2-610b-db2a-16589df90faf@fb.com>
Date: Wed, 13 May 2020 16:04:26 -0700
From: Yonghong Song <yhs@...com>
To: Alan Maguire <alan.maguire@...cle.com>, <ast@...nel.org>,
<daniel@...earbox.net>, <bpf@...r.kernel.org>
CC: <joe@...ches.com>, <linux@...musvillemoes.dk>,
<arnaldo.melo@...il.com>, <kafai@...com>, <songliubraving@...com>,
<andriin@...com>, <john.fastabend@...il.com>,
<kpsingh@...omium.org>, <linux-kernel@...r.kernel.org>,
<netdev@...r.kernel.org>
Subject: Re: [PATCH v2 bpf-next 2/7] bpf: move to generic BTF show support,
apply it to seq files/strings
On 5/11/20 10:56 PM, Alan Maguire wrote:
> generalize the "seq_show" seq file support in btf.c to support
> a generic show callback of which we support two instances; the
> current seq file show, and a show with snprintf() behaviour which
> instead writes the type data to a supplied string.
>
> Both classes of show function call btf_type_show() with different
> targets; the seq file or the string to be written. In the string
> case we need to track additional data - length left in string to write
> and length to return that we would have written (a la snprintf).
>
> By default show will display type information, field members and
> their types and values etc, and the information is indented
> based upon structure depth.
>
> Show however supports flags which modify its behaviour:
>
> BTF_SHOW_COMPACT - suppress newline/indent.
> BTF_SHOW_NONAME - suppress show of type and member names.
> BTF_SHOW_PTR_RAW - do not obfuscate pointer values.
> BTF_SHOW_ZERO - show zeroed values (by default they are not shown).
>
> Signed-off-by: Alan Maguire <alan.maguire@...cle.com>
> ---
> include/linux/btf.h | 33 +++
> kernel/bpf/btf.c | 759 +++++++++++++++++++++++++++++++++++++++++++++-------
> 2 files changed, 690 insertions(+), 102 deletions(-)
>
> diff --git a/include/linux/btf.h b/include/linux/btf.h
> index 5c1ea99..d571125 100644
> --- a/include/linux/btf.h
> +++ b/include/linux/btf.h
> @@ -13,6 +13,7 @@
> struct btf_member;
> struct btf_type;
> union bpf_attr;
> +struct btf_show;
>
> extern const struct file_operations btf_fops;
>
> @@ -46,8 +47,40 @@ int btf_get_info_by_fd(const struct btf *btf,
> const struct btf_type *btf_type_id_size(const struct btf *btf,
> u32 *type_id,
> u32 *ret_size);
> +
> +/*
> + * Options to control show behaviour.
> + * - BTF_SHOW_COMPACT: no formatting around type information
> + * - BTF_SHOW_NONAME: no struct/union member names/types
> + * - BTF_SHOW_PTR_RAW: show raw (unobfuscated) pointer values;
> + * equivalent to %px.
> + * - BTF_SHOW_ZERO: show zero-valued struct/union members; they
> + * are not displayed by default
> + */
> +#define BTF_SHOW_COMPACT (1ULL << 0)
> +#define BTF_SHOW_NONAME (1ULL << 1)
> +#define BTF_SHOW_PTR_RAW (1ULL << 2)
> +#define BTF_SHOW_ZERO (1ULL << 3)
> +
> void btf_type_seq_show(const struct btf *btf, u32 type_id, void *obj,
> struct seq_file *m);
> +
> +/*
> + * Copy len bytes of string representation of obj of BTF type_id into buf.
> + *
> + * @btf: struct btf object
> + * @type_id: type id of type obj points to
> + * @obj: pointer to typed data
> + * @buf: buffer to write to
> + * @len: maximum length to write to buf
> + * @flags: show options (see above)
> + *
> + * Return: length that would have been/was copied as per snprintf, or
> + * negative error.
> + */
> +int btf_type_snprintf_show(const struct btf *btf, u32 type_id, void *obj,
> + char *buf, int len, u64 flags);
> +
> int btf_get_fd_by_id(u32 id);
> u32 btf_id(const struct btf *btf);
> bool btf_member_is_reg_int(const struct btf *btf, const struct btf_type *s,
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index dcd2331..edf6455 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -281,6 +281,32 @@ static const char *btf_type_str(const struct btf_type *t)
> return btf_kind_str[BTF_INFO_KIND(t->info)];
> }
>
> +/*
> + * Common data to all BTF show operations. Private show functions can add
> + * their own data to a structure containing a struct btf_show and consult it
> + * in the show callback. See btf_type_show() below.
> + */
> +struct btf_show {
> + u64 flags;
> + void *target; /* target of show operation (seq file, buffer) */
> + void (*showfn)(struct btf_show *show, const char *fmt, ...);
> + const struct btf *btf;
> + /* below are used during iteration */
> + struct {
> + u8 depth;
> + u8 depth_shown;
> + u8 depth_check;
I have some difficulties to understand the relationship between
the above three variables. Could you add some comments here?
> + u8 array_member:1,
> + array_terminated:1;
> + u16 array_encoding;
> + u32 type_id;
> + const struct btf_type *type;
> + const struct btf_member *member;
> + char name[KSYM_NAME_LEN]; /* scratch space for name */
> + char type_name[KSYM_NAME_LEN]; /* scratch space for type */
KSYM_NAME_LEN is for symbol name, not for type name. But I guess in
kernel we probably do not have > 128 bytes type name so we should be
okay here.
> + } state;
> +};
> +
> struct btf_kind_operations {
> s32 (*check_meta)(struct btf_verifier_env *env,
> const struct btf_type *t,
> @@ -297,9 +323,9 @@ struct btf_kind_operations {
> const struct btf_type *member_type);
> void (*log_details)(struct btf_verifier_env *env,
> const struct btf_type *t);
> - void (*seq_show)(const struct btf *btf, const struct btf_type *t,
> + void (*show)(const struct btf *btf, const struct btf_type *t,
> u32 type_id, void *data, u8 bits_offsets,
> - struct seq_file *m);
> + struct btf_show *show);
> };
>
> static const struct btf_kind_operations * const kind_ops[NR_BTF_KINDS];
> @@ -676,6 +702,340 @@ bool btf_member_is_reg_int(const struct btf *btf, const struct btf_type *s,
> return true;
> }
>
> +/* Similar to btf_type_skip_modifiers() but does not skip typedefs. */
> +static inline
> +const struct btf_type *btf_type_skip_qualifiers(const struct btf *btf, u32 id)
> +{
> + const struct btf_type *t = btf_type_by_id(btf, id);
> +
> + while (btf_type_is_modifier(t) &&
> + BTF_INFO_KIND(t->info) != BTF_KIND_TYPEDEF) {
> + id = t->type;
> + t = btf_type_by_id(btf, t->type);
> + }
> +
> + return t;
> +}
> +
> +#define BTF_SHOW_MAX_ITER 10
> +
> +#define BTF_KIND_BIT(kind) (1ULL << kind)
> +
> +static inline const char *btf_show_type_name(struct btf_show *show,
> + const struct btf_type *t)
> +{
> + const char *array_suffixes = "[][][][][][][][][][]";
Add a comment here saying length BTF_SHOW_MAX_ITER * 2
so later on if somebody changes the BTF_SHOW_MAX_ITER from 10 to 12,
it won't miss here?
> + const char *array_suffix = &array_suffixes[strlen(array_suffixes)];
> + const char *ptr_suffixes = "**********";
The same here.
> + const char *ptr_suffix = &ptr_suffixes[strlen(ptr_suffixes)];
> + const char *type_name = NULL, *prefix = "", *parens = "";
> + const struct btf_array *array;
> + u32 id = show->state.type_id;
> + bool allow_anon = true;
> + u64 kinds = 0;
> + int i;
> +
> + show->state.type_name[0] = '\0';
> +
> + /*
> + * Start with type_id, as we have have resolved the struct btf_type *
> + * via btf_modifier_show() past the parent typedef to the child
> + * struct, int etc it is defined as. In such cases, the type_id
> + * still represents the starting type while the the struct btf_type *
> + * in our show->state points at the resolved type of the typedef.
> + */
> + t = btf_type_by_id(show->btf, id);
> + if (!t)
> + return show->state.type_name;
> +
> + /*
> + * The goal here is to build up the right number of pointer and
> + * array suffixes while ensuring the type name for a typedef
> + * is represented. Along the way we accumulate a list of
> + * BTF kinds we have encountered, since these will inform later
> + * display; for example, pointer types will not require an
> + * opening "{" for struct, we will just display the pointer value.
> + *
> + * We also want to accumulate the right number of pointer or array
> + * indices in the format string while iterating until we get to
> + * the typedef/pointee/array member target type.
> + *
> + * We start by pointing at the end of pointer and array suffix
> + * strings; as we accumulate pointers and arrays we move the pointer
> + * or array string backwards so it will show the expected number of
> + * '*' or '[]' for the type. BTF_SHOW_MAX_ITER of nesting of pointers
> + * and/or arrays and typedefs are supported as a precaution.
> + *
> + * We also want to get typedef name while proceeding to resolve
> + * type it points to so that we can add parentheses if it is a
> + * "typedef struct" etc.
> + */
> + for (i = 0; i < BTF_SHOW_MAX_ITER; i++) {
> +
> + switch (BTF_INFO_KIND(t->info)) {
> + case BTF_KIND_TYPEDEF:
> + if (!type_name)
> + type_name = btf_name_by_offset(show->btf,
> + t->name_off);
> + kinds |= BTF_KIND_BIT(BTF_KIND_TYPEDEF);
> + id = t->type;
> + break;
> + case BTF_KIND_ARRAY:
> + kinds |= BTF_KIND_BIT(BTF_KIND_ARRAY);
> + parens = "[";
> + array = btf_type_array(t);
> + if (!array)
> + return show->state.type_name;
> + if (!t)
> + return show->state.type_name;
> + if (array_suffix > array_suffixes)
> + array_suffix -= 2;
> + id = array->type;
> + break;
> + case BTF_KIND_PTR:
> + kinds |= BTF_KIND_BIT(BTF_KIND_PTR);
> + if (ptr_suffix > ptr_suffixes)
> + ptr_suffix -= 1;
> + id = t->type;
> + break;
> + default:
> + id = 0;
> + break;
> + }
> + if (!id)
> + break;
> + t = btf_type_skip_qualifiers(show->btf, id);
> + if (!t)
> + return show->state.type_name;
> + }
Do we do pointer tracing here? For example
struct t {
int *m[5];
}
When trying to access memory, the above code may go through
ptr->array and out of loop when hitting array element type "int"?
> + /* We may not be able to represent this type; bail to be safe */
> + if (i == BTF_SHOW_MAX_ITER)
> + return show->state.type_name;
> +
> + if (!type_name)
> + type_name = btf_name_by_offset(show->btf, t->name_off);
> +
> + switch (BTF_INFO_KIND(t->info)) {
> + case BTF_KIND_STRUCT:
> + case BTF_KIND_UNION:
> + prefix = BTF_INFO_KIND(t->info) == BTF_KIND_STRUCT ?
> + "struct" : "union";
> + /* if it's an array of struct/union, parens is already set */
> + if (!(kinds & (BTF_KIND_BIT(BTF_KIND_ARRAY))))
> + parens = "{";
> + break;
> + case BTF_KIND_ENUM:
> + prefix = "enum";
> + break;
> + default:
> + allow_anon = false;
> + break;
> + }
> +
> + /* pointer does not require parens */
> + if (kinds & BTF_KIND_BIT(BTF_KIND_PTR))
> + parens = "";
> + /* typedef does not require struct/union/enum prefix */
> + if (kinds & BTF_KIND_BIT(BTF_KIND_TYPEDEF))
> + prefix = "";
> +
> + if (!type_name || strlen(type_name) == 0) {
> + if (allow_anon)
> + type_name = "";
> + else
> + return show->state.type_name;
> + }
> +
> + /* Even if we don't want type name info, we want parentheses etc */
> + if (show->flags & BTF_SHOW_NONAME)
> + snprintf(show->state.type_name, sizeof(show->state.type_name),
> + "%s", parens);
> + else
> + snprintf(show->state.type_name, sizeof(show->state.type_name),
> + "(%s%s%s%s%s%s)%s",
> + prefix,
> + strlen(prefix) > 0 && strlen(type_name) > 0 ? " " : "",
> + type_name,
> + strlen(ptr_suffix) > 0 ? " " : "", ptr_suffix,
> + array_suffix, parens);
> +
> + return show->state.type_name;
> +}
> +
> +static inline const char *btf_show_name(struct btf_show *show)
> +{
> + const struct btf_type *t = show->state.type;
> + const struct btf_member *m = show->state.member;
> + const char *member = NULL;
> + const char *type = "";
> +
> + show->state.name[0] = '\0';
> +
> + if ((!m && !t) || show->state.array_member)
> + return show->state.name;
> +
> + if (m)
> + t = btf_type_skip_qualifiers(show->btf, m->type);
> +
> + if (t) {
> + type = btf_show_type_name(show, t);
> + if (!type)
> + return show->state.name;
> + }
> +
> + if (m && !(show->flags & BTF_SHOW_NONAME)) {
> + member = btf_name_by_offset(show->btf, m->name_off);
> + if (member && strlen(member) > 0) {
> + snprintf(show->state.name, sizeof(show->state.name),
> + ".%s = %s", member, type);
> + return show->state.name;
> + }
> + }
> +
> + snprintf(show->state.name, sizeof(show->state.name), "%s", type);
> +
> + return show->state.name;
> +}
> +
> +#define btf_show(show, ...) \
> + do { \
> + if (!show->state.depth_check) \
As I mentioned above, some comments will be good to understand here.
> + show->showfn(show, __VA_ARGS__); \
> + } while (0)
> +
> +static inline const char *__btf_show_indent(struct btf_show *show)
> +{
> + const char *indents = " ";
> + const char *indent = &indents[strlen(indents)];
> +
> + if ((indent - show->state.depth) >= indents)
> + return indent - show->state.depth;
> + return indents;
> +}
> +
> +#define btf_show_indent(show) \
> + ((show->flags & BTF_SHOW_COMPACT) ? "" : __btf_show_indent(show))
> +
> +#define btf_show_newline(show) \
> + ((show->flags & BTF_SHOW_COMPACT) ? "" : "\n")
> +
> +#define btf_show_delim(show) \
> + (show->state.depth == 0 ? "" : \
> + ((show->flags & BTF_SHOW_COMPACT) && show->state.type && \
> + BTF_INFO_KIND(show->state.type->info) == BTF_KIND_UNION) ? "|" : ",")
> +
> +#define btf_show_type_value(show, fmt, value) \
> + do { \
> + if ((value) != 0 || (show->flags & BTF_SHOW_ZERO) || \
> + show->state.depth == 0) { \
> + btf_show(show, "%s%s" fmt "%s%s", \
> + btf_show_indent(show), \
> + btf_show_name(show), \
> + value, btf_show_delim(show), \
> + btf_show_newline(show)); \
> + if (show->state.depth > show->state.depth_shown) \
> + show->state.depth_shown = show->state.depth; \
> + } \
> + } while (0)
> +
> +#define btf_show_type_values(show, fmt, ...) \
> + do { \
> + btf_show(show, "%s%s" fmt "%s%s", btf_show_indent(show), \
> + btf_show_name(show), \
> + __VA_ARGS__, btf_show_delim(show), \
> + btf_show_newline(show)); \
> + if (show->state.depth > show->state.depth_shown) \
> + show->state.depth_shown = show->state.depth; \
> + } while (0)
> +
[...]
>
> static int btf_array_check_member(struct btf_verifier_env *env,
> @@ -2104,28 +2489,87 @@ static void btf_array_log(struct btf_verifier_env *env,
> array->type, array->index_type, array->nelems);
> }
>
> -static void btf_array_seq_show(const struct btf *btf, const struct btf_type *t,
> - u32 type_id, void *data, u8 bits_offset,
> - struct seq_file *m)
> +static void __btf_array_show(const struct btf *btf, const struct btf_type *t,
> + u32 type_id, void *data, u8 bits_offset,
> + struct btf_show *show)
> {
> const struct btf_array *array = btf_type_array(t);
> const struct btf_kind_operations *elem_ops;
> const struct btf_type *elem_type;
> - u32 i, elem_size, elem_type_id;
> + u32 i, elem_size = 0, elem_type_id;
> + u16 encoding = 0;
>
> elem_type_id = array->type;
> - elem_type = btf_type_id_size(btf, &elem_type_id, &elem_size);
> + elem_type = btf_type_skip_modifiers(btf, elem_type_id, NULL);
> + if (elem_type && btf_type_has_size(elem_type))
> + elem_size = elem_type->size;
> +
> + if (elem_type && btf_type_is_int(elem_type)) {
> + u32 int_type = btf_type_int(elem_type);
> +
> + encoding = BTF_INT_ENCODING(int_type);
> +
> + /*
> + * BTF_INT_CHAR encoding never seems to be set for
> + * char arrays, so if size is 1 and element is
> + * printable as a char, we'll do that.
> + */
> + if (elem_size == 1) > + encoding = BTF_INT_CHAR;
Some char array may be printable and some may not be printable,
how did you differentiate this?
> + }
> +
> + btf_show_start_array_type(show, t, type_id, encoding);
> +
> + if (!elem_type)
> + goto out;
> elem_ops = btf_type_ops(elem_type);
> - seq_puts(m, "[");
> +
> for (i = 0; i < array->nelems; i++) {
> - if (i)
> - seq_puts(m, ",");
>
> - elem_ops->seq_show(btf, elem_type, elem_type_id, data,
> - bits_offset, m);
> + btf_show_start_array_member(show);
> +
> + elem_ops->show(btf, elem_type, elem_type_id, data,
> + bits_offset, show);
> data += elem_size;
> +
> + btf_show_end_array_member(show);
> +
> + if (show->state.array_terminated)
> + break;
> }
> - seq_puts(m, "]");
> +out:
> + btf_show_end_array_type(show);
> +}
> +
[...]
Powered by blists - more mailing lists