* [lttng-dev] [PATCH] Avoid calling caa_container_of on NULL pointer in cds_lfhash macros
@ 2023-06-22 10:26 Ondřej Surý via lttng-dev
2023-06-22 10:45 ` Ondřej Surý via lttng-dev
0 siblings, 1 reply; 3+ messages in thread
From: Ondřej Surý via lttng-dev @ 2023-06-22 10:26 UTC (permalink / raw)
To: lttng-dev
The cds_lfht_for_each_entry and cds_lfht_for_each_entry_duplicate macros
would call caa_container_of() macro on NULL pointer. This is not a
problem under normal circumstances as the check in the for loop fails
and the loop-statement is not called with invalid (pos) value.
However AddressSanitizer doesn't like that and complains about this:
runtime error: applying non-zero offset 18446744073709551056 to null pointer
Move the cds_lfht_iter_get_node(iter) != NULL from the cond-expression
of the for loop into both init-clause and iteration-expression as
conditional operator and check for (pos) value in the cond-expression
instead.
Signed-off-by: Ondřej Surý <ondrej@sury.org>
---
include/urcu/rculfhash.h | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/include/urcu/rculfhash.h b/include/urcu/rculfhash.h
index fbd33cc..aafc455 100644
--- a/include/urcu/rculfhash.h
+++ b/include/urcu/rculfhash.h
@@ -546,22 +546,22 @@ void cds_lfht_resize(struct cds_lfht *ht, unsigned long new_size);
#define cds_lfht_for_each_entry(ht, iter, pos, member) \
for (cds_lfht_first(ht, iter), \
- pos = caa_container_of(cds_lfht_iter_get_node(iter), \
- __typeof__(*(pos)), member); \
- cds_lfht_iter_get_node(iter) != NULL; \
+ pos = (cds_lfht_iter_get_node(iter) != NULL ? caa_container_of(cds_lfht_iter_get_node(iter), \
+ __typeof__(*(pos)), member) : NULL); \
+ pos != NULL; \
cds_lfht_next(ht, iter), \
- pos = caa_container_of(cds_lfht_iter_get_node(iter), \
- __typeof__(*(pos)), member))
+ pos = (cds_lfht_iter_get_node(iter) != NULL ? caa_container_of(cds_lfht_iter_get_node(iter), \
+ __typeof__(*(pos)), member) : NULL)
#define cds_lfht_for_each_entry_duplicate(ht, hash, match, key, \
iter, pos, member) \
for (cds_lfht_lookup(ht, hash, match, key, iter), \
- pos = caa_container_of(cds_lfht_iter_get_node(iter), \
- __typeof__(*(pos)), member); \
- cds_lfht_iter_get_node(iter) != NULL; \
+ pos = (cds_lfht_iter_get_node(iter) != NULL ? caa_container_of(cds_lfht_iter_get_node(iter), \
+ __typeof__(*(pos)), member) : NULL); \
+ pos != NULL; \
cds_lfht_next_duplicate(ht, match, key, iter), \
- pos = caa_container_of(cds_lfht_iter_get_node(iter), \
- __typeof__(*(pos)), member))
+ pos = (cds_lfht_iter_get_node(iter) != NULL ? caa_container_of(cds_lfht_iter_get_node(iter), \
+ __typeof__(*(pos)), member) : NULL)
#ifdef __cplusplus
}
--
2.39.2
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
^ permalink raw reply related [flat|nested] 3+ messages in thread* [lttng-dev] [PATCH] Avoid calling caa_container_of on NULL pointer in cds_lfhash macros
2023-06-22 10:26 [lttng-dev] [PATCH] Avoid calling caa_container_of on NULL pointer in cds_lfhash macros Ondřej Surý via lttng-dev
@ 2023-06-22 10:45 ` Ondřej Surý via lttng-dev
2023-06-22 14:04 ` Mathieu Desnoyers via lttng-dev
0 siblings, 1 reply; 3+ messages in thread
From: Ondřej Surý via lttng-dev @ 2023-06-22 10:45 UTC (permalink / raw)
To: lttng-dev
(Sorry, I missed closing brackets in both macros, so resending fixed patch...)
The cds_lfht_for_each_entry and cds_lfht_for_each_entry_duplicate macros
would call caa_container_of() macro on NULL pointer. This is not a
problem under normal circumstances as the check in the for loop fails
and the loop-statement is not called with invalid (pos) value.
However AddressSanitizer doesn't like that and complains about this:
runtime error: applying non-zero offset 18446744073709551056 to null pointer
Move the cds_lfht_iter_get_node(iter) != NULL from the cond-expression
of the for loop into both init-clause and iteration-expression as
conditional operator and check for (pos) value in the cond-expression
instead.
Signed-off-by: Ondřej Surý <ondrej@sury.org>
---
include/urcu/rculfhash.h | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/include/urcu/rculfhash.h b/include/urcu/rculfhash.h
index fbd33cc..64cc18f 100644
--- a/include/urcu/rculfhash.h
+++ b/include/urcu/rculfhash.h
@@ -546,22 +546,22 @@ void cds_lfht_resize(struct cds_lfht *ht, unsigned long new_size);
#define cds_lfht_for_each_entry(ht, iter, pos, member) \
for (cds_lfht_first(ht, iter), \
- pos = caa_container_of(cds_lfht_iter_get_node(iter), \
- __typeof__(*(pos)), member); \
- cds_lfht_iter_get_node(iter) != NULL; \
+ pos = (cds_lfht_iter_get_node(iter) != NULL ? caa_container_of(cds_lfht_iter_get_node(iter), \
+ __typeof__(*(pos)), member) : NULL); \
+ pos != NULL; \
cds_lfht_next(ht, iter), \
- pos = caa_container_of(cds_lfht_iter_get_node(iter), \
- __typeof__(*(pos)), member))
+ pos = (cds_lfht_iter_get_node(iter) != NULL ? caa_container_of(cds_lfht_iter_get_node(iter), \
+ __typeof__(*(pos)), member) : NULL))
#define cds_lfht_for_each_entry_duplicate(ht, hash, match, key, \
iter, pos, member) \
for (cds_lfht_lookup(ht, hash, match, key, iter), \
- pos = caa_container_of(cds_lfht_iter_get_node(iter), \
- __typeof__(*(pos)), member); \
- cds_lfht_iter_get_node(iter) != NULL; \
+ pos = (cds_lfht_iter_get_node(iter) != NULL ? caa_container_of(cds_lfht_iter_get_node(iter), \
+ __typeof__(*(pos)), member) : NULL); \
+ pos != NULL; \
cds_lfht_next_duplicate(ht, match, key, iter), \
- pos = caa_container_of(cds_lfht_iter_get_node(iter), \
- __typeof__(*(pos)), member))
+ pos = (cds_lfht_iter_get_node(iter) != NULL ? caa_container_of(cds_lfht_iter_get_node(iter), \
+ __typeof__(*(pos)), member) : NULL))
#ifdef __cplusplus
}
--
2.39.2
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [lttng-dev] [PATCH] Avoid calling caa_container_of on NULL pointer in cds_lfhash macros
2023-06-22 10:45 ` Ondřej Surý via lttng-dev
@ 2023-06-22 14:04 ` Mathieu Desnoyers via lttng-dev
0 siblings, 0 replies; 3+ messages in thread
From: Mathieu Desnoyers via lttng-dev @ 2023-06-22 14:04 UTC (permalink / raw)
To: Ondřej Surý, lttng-dev
On 6/22/23 06:45, Ondřej Surý via lttng-dev wrote:
> (Sorry, I missed closing brackets in both macros, so resending fixed patch...)
>
> The cds_lfht_for_each_entry and cds_lfht_for_each_entry_duplicate macros
> would call caa_container_of() macro on NULL pointer. This is not a
> problem under normal circumstances as the check in the for loop fails
> and the loop-statement is not called with invalid (pos) value.
>
> However AddressSanitizer doesn't like that and complains about this:
>
> runtime error: applying non-zero offset 18446744073709551056 to null pointer
>
> Move the cds_lfht_iter_get_node(iter) != NULL from the cond-expression
> of the for loop into both init-clause and iteration-expression as
> conditional operator and check for (pos) value in the cond-expression
> instead.
I've taken the liberty to reimplement this with a new helper "cds_lfht_entry".
Can you review and try the following commits please ?
https://review.lttng.org/c/userspace-rcu/+/10445 compiler.h: Introduce caa_unqual_scalar_typeof
https://review.lttng.org/c/userspace-rcu/+/10446 Avoid calling caa_container_of on NULL pointer in cds_lfht macros
Thanks!
Mathieu
>
> Signed-off-by: Ondřej Surý <ondrej@sury.org>
> ---
> include/urcu/rculfhash.h | 20 ++++++++++----------
> 1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/include/urcu/rculfhash.h b/include/urcu/rculfhash.h
> index fbd33cc..64cc18f 100644
> --- a/include/urcu/rculfhash.h
> +++ b/include/urcu/rculfhash.h
> @@ -546,22 +546,22 @@ void cds_lfht_resize(struct cds_lfht *ht, unsigned long new_size);
>
> #define cds_lfht_for_each_entry(ht, iter, pos, member) \
> for (cds_lfht_first(ht, iter), \
> - pos = caa_container_of(cds_lfht_iter_get_node(iter), \
> - __typeof__(*(pos)), member); \
> - cds_lfht_iter_get_node(iter) != NULL; \
> + pos = (cds_lfht_iter_get_node(iter) != NULL ? caa_container_of(cds_lfht_iter_get_node(iter), \
> + __typeof__(*(pos)), member) : NULL); \
> + pos != NULL; \
> cds_lfht_next(ht, iter), \
> - pos = caa_container_of(cds_lfht_iter_get_node(iter), \
> - __typeof__(*(pos)), member))
> + pos = (cds_lfht_iter_get_node(iter) != NULL ? caa_container_of(cds_lfht_iter_get_node(iter), \
> + __typeof__(*(pos)), member) : NULL))
>
> #define cds_lfht_for_each_entry_duplicate(ht, hash, match, key, \
> iter, pos, member) \
> for (cds_lfht_lookup(ht, hash, match, key, iter), \
> - pos = caa_container_of(cds_lfht_iter_get_node(iter), \
> - __typeof__(*(pos)), member); \
> - cds_lfht_iter_get_node(iter) != NULL; \
> + pos = (cds_lfht_iter_get_node(iter) != NULL ? caa_container_of(cds_lfht_iter_get_node(iter), \
> + __typeof__(*(pos)), member) : NULL); \
> + pos != NULL; \
> cds_lfht_next_duplicate(ht, match, key, iter), \
> - pos = caa_container_of(cds_lfht_iter_get_node(iter), \
> - __typeof__(*(pos)), member))
> + pos = (cds_lfht_iter_get_node(iter) != NULL ? caa_container_of(cds_lfht_iter_get_node(iter), \
> + __typeof__(*(pos)), member) : NULL))
>
> #ifdef __cplusplus
> }
--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-06-22 14:04 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-22 10:26 [lttng-dev] [PATCH] Avoid calling caa_container_of on NULL pointer in cds_lfhash macros Ondřej Surý via lttng-dev
2023-06-22 10:45 ` Ondřej Surý via lttng-dev
2023-06-22 14:04 ` Mathieu Desnoyers via lttng-dev
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).