| 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: <acc8a12f-e9c3-4c1d-9e7f-9dd9be473292@oracle.com>
Date: Tue, 26 Aug 2025 15:54:13 +0300
From: Claudiu Zissulescu-Ianculescu <claudiu.zissulescu-ianculescu@...cle.com>
To: Kees Cook <kees@...nel.org>
Cc: Andrew Pinski <andrew.pinski@....qualcomm.com>,
Qing Zhao <qing.zhao@...cle.com>, gcc-patches@....gnu.org,
linux-hardening@...r.kernel.org
Subject: Re: [PATCH v2] Fix sanitizer attribute infrastructure to use standard
TREE_LIST format [PR113264]
Hi Kees,
I think the tests are missing.
Best wishes,
Claudiu
On 8/26/25 9:17 AM, Kees Cook wrote:
> The __attribute__((__copy__)) functionality was crashing when copying
> sanitizer-related attributes because these attributes violated the standard
> GCC attribute infrastructure by storing INTEGER_CST values directly instead
> of wrapping them in TREE_LIST like all other attributes.
>
> Wrap sanitizer attributes INTEGER_CST values in TREE_LIST structures
> to follow the same pattern as other attributes. This eliminates the
> copy_list() crashes when copying sanitizer attributes:
>
> test.c:4:1: internal compiler error: tree check: expected tree that contains ‘common’ structure, have ‘integer_cst’ in copy_list, at tree.cc:1427
> 4 | __attribute__((__copy__(__tanh)));
> | ^~~~~~~~~~~~~
> 0x859d06 tree_contains_struct_check_failed(tree_node const*, tree_node_structure_enum, char const*, int, char const*)
> ../../gcc/gcc/tree.cc:9126
> 0x860f78 contains_struct_check(tree_node*, tree_node_structure_enum, char const*, int, char const*)
> ../../gcc/gcc/tree.h:3748
> 0x860f78 copy_list(tree_node*)
> ../../gcc/gcc/tree.cc:1427
> 0xa755a5 handle_copy_attribute
> ../../gcc/gcc/c-family/c-attribs.cc:3077
>
> gcc/c-family/ChangeLog:
>
> PR c/113264
> * c-attribs.cc (add_no_sanitize_value): Store INTEGER_CST values
> wrapped in TREE_LIST following standard attribute conventions.
> (handle_no_sanitize_attribute): Handle both original string
> arguments and copied INTEGER_CST values from TREE_LIST format.
>
> gcc/ChangeLog:
>
> PR c/113264
> * asan.h (sanitize_flags_p): Extract sanitizer flags from
> TREE_LIST wrapper instead of directly from INTEGER_CST.
>
> gcc/d/ChangeLog:
>
> PR c/113264
> * d-attribs.cc (d_handle_no_sanitize_attribute): Store INTEGER_CST
> values wrapped in TREE_LIST following standard conventions.
>
> gcc/testsuite/ChangeLog:
>
> PR c/113264
> * gcc.dg/pr113264.c: New test.
> * gcc.dg/pr113264-asan-no-instrumentation.c: New test validating
> that copied sanitizer attributes prevent ASAN instrumentation.
>
> Signed-off-by: Kees Cook <kees@...nel.org>
> ---
> v2: Rebased on to latest upstream, which has sanitize_code_type now.
> Cc: Indu Bhagat <indu.bhagat@...cle.com>
> Cc: Claudiu Zissulescu <claudiu.zissulescu-ianculescu@...cle.com>
> Cc: Andrew Pinski <andrew.pinski@....qualcomm.com>
> ---
> gcc/asan.h | 8 ++++++-
> gcc/c-family/c-attribs.cc | 49 +++++++++++++++++++++++++++------------
> gcc/d/d-attribs.cc | 17 ++++++++++----
> 3 files changed, 54 insertions(+), 20 deletions(-)
>
> diff --git a/gcc/asan.h b/gcc/asan.h
> index a24562f67a29..f5dc3d7ceb8d 100644
> --- a/gcc/asan.h
> +++ b/gcc/asan.h
> @@ -253,7 +253,13 @@ sanitize_flags_p (sanitize_code_type flag,
> {
> tree value = lookup_attribute ("no_sanitize", DECL_ATTRIBUTES (fn));
> if (value)
> - result_flags &= ~tree_to_uhwi (TREE_VALUE (value));
> + {
> + /* Extract the INTEGER_CST from the TREE_LIST wrapper. */
> + tree attr_args = TREE_VALUE (value);
> + gcc_assert (attr_args && TREE_CODE (attr_args) == TREE_LIST);
> + sanitize_code_type no_sanitize_flags = tree_to_sanitize_code_type (TREE_VALUE (attr_args));
> + result_flags &= ~no_sanitize_flags;
> + }
> }
>
> return result_flags;
> diff --git a/gcc/c-family/c-attribs.cc b/gcc/c-family/c-attribs.cc
> index 1e3a94ed9493..e629601579ef 100644
> --- a/gcc/c-family/c-attribs.cc
> +++ b/gcc/c-family/c-attribs.cc
> @@ -1425,20 +1425,27 @@ add_no_sanitize_value (tree node, sanitize_code_type flags)
> tree attr = lookup_attribute ("no_sanitize", DECL_ATTRIBUTES (node));
> if (attr)
> {
> - sanitize_code_type old_value =
> - tree_to_sanitize_code_type (TREE_VALUE (attr));
> + /* Extract the INTEGER_CST from the TREE_LIST wrapper. */
> + tree attr_args = TREE_VALUE (attr);
> + gcc_assert (attr_args && TREE_CODE (attr_args) == TREE_LIST);
> + sanitize_code_type old_value = tree_to_sanitize_code_type (TREE_VALUE (attr_args));
> +
> flags |= old_value;
>
> if (flags == old_value)
> return;
>
> - TREE_VALUE (attr) = build_int_cst (uint64_type_node, flags);
> + tree new_value = build_tree_list (NULL_TREE,
> + build_int_cst (uint64_type_node, flags));
> + TREE_VALUE (attr) = new_value;
> }
> else
> - DECL_ATTRIBUTES (node)
> - = tree_cons (get_identifier ("no_sanitize"),
> - build_int_cst (uint64_type_node, flags),
> - DECL_ATTRIBUTES (node));
> + {
> + tree attr_value = build_tree_list (NULL_TREE,
> + build_int_cst (uint64_type_node, flags));
> + DECL_ATTRIBUTES (node) = tree_cons (get_identifier ("no_sanitize"), attr_value,
> + DECL_ATTRIBUTES (node));
> + }
> }
>
> /* Handle a "no_sanitize" attribute; arguments as in
> @@ -1456,17 +1463,29 @@ handle_no_sanitize_attribute (tree *node, tree name, tree args, int,
> return NULL_TREE;
> }
>
> - for (; args; args = TREE_CHAIN (args))
> + /* Handle both original string arguments and copied attributes that
> + have already been processed into INTEGER_CST wrapped in TREE_LIST. */
> + if (args && TREE_CODE (args) == TREE_LIST
> + && TREE_VALUE (args) && TREE_CODE (TREE_VALUE (args)) == INTEGER_CST)
> {
> - tree id = TREE_VALUE (args);
> - if (TREE_CODE (id) != STRING_CST)
> + /* This is a copied attribute with the flags already processed. */
> + flags = tree_to_sanitize_code_type (TREE_VALUE (args));
> + }
> + else
> + {
> + /* Process original string arguments. */
> + for (; args; args = TREE_CHAIN (args))
> {
> - error ("%qE argument not a string", name);
> - return NULL_TREE;
> - }
> + tree id = TREE_VALUE (args);
> + if (TREE_CODE (id) != STRING_CST)
> + {
> + error ("%qE argument not a string", name);
> + return NULL_TREE;
> + }
>
> - char *string = ASTRDUP (TREE_STRING_POINTER (id));
> - flags |= parse_no_sanitize_attribute (string);
> + char *string = ASTRDUP (TREE_STRING_POINTER (id));
> + flags |= parse_no_sanitize_attribute (string);
> + }
> }
>
> add_no_sanitize_value (*node, flags);
> diff --git a/gcc/d/d-attribs.cc b/gcc/d/d-attribs.cc
> index 53aea5e2e904..342702b05acb 100644
> --- a/gcc/d/d-attribs.cc
> +++ b/gcc/d/d-attribs.cc
> @@ -1424,17 +1424,26 @@ d_handle_no_sanitize_attribute (tree *node, tree name, tree args, int,
> merge existing flags if no_sanitize was previously handled. */
> if (tree attr = lookup_attribute ("no_sanitize", DECL_ATTRIBUTES (*node)))
> {
> - sanitize_code_type old_value =
> - tree_to_sanitize_code_type (TREE_VALUE (attr));
> + /* Extract the INTEGER_CST from the TREE_LIST wrapper. */
> + tree attr_args = TREE_VALUE (attr);
> + gcc_assert (attr_args && TREE_CODE (attr_args) == TREE_LIST);
> + sanitize_code_type old_value = tree_to_sanitize_code_type (TREE_VALUE (attr_args));
> +
> flags |= old_value;
>
> if (flags != old_value)
> - TREE_VALUE (attr) = build_int_cst (d_ulong_type, flags);
> + {
> + tree new_value = build_tree_list (NULL_TREE,
> + build_int_cst (d_ulong_type, flags));
> + TREE_VALUE (attr) = new_value;
> + }
> }
> else
> {
> + tree attr_value = build_tree_list (NULL_TREE,
> + build_int_cst (d_ulong_type, flags));
> DECL_ATTRIBUTES (*node) = tree_cons (get_identifier ("no_sanitize"),
> - build_int_cst (d_ulong_type, flags),
> + attr_value,
> DECL_ATTRIBUTES (*node));
> }
>
Powered by blists - more mailing lists