Archive-Team
/
zfs
2
0
Fork 0
Code Issues Pull Requests Packages Projects Releases 103 Wiki Activity

Fix assertions in crypto reference helpers 

The assertions are racy and the use of `membar_exit()` did nothing to
fix that.

The helpers use atomic functions, so we cleverly get values from the
atomics that we can use to ensure that the assertions operate on the
correct values.

We also use `membar_producer()` prior to decrementing reference counts
so that operations that happened prior to a decrement to 0 will be
guaranteed to happen before the decrement on architectures that reorder
atomics.

This also slightly improves performance by eliminating unnecessary
reads, although I doubt it would be measurable in any benchmark.

Reviewed-by: Mateusz Guzik <mjguzik@gmail.com>
Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Closes #13880
This commit is contained in:
Richard Yao 2022-09-15 16:24:00 -04:00 committed by GitHub
parent dc2fe24ca2
commit e949d36040
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 25 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

42
module/icp/include/sys/crypto/impl.h
View File

@ -126,28 +126,26 @@ typedef struct kcf_provider_desc {
crypto_provider_id_t pd_prov_id;
} kcf_provider_desc_t;
/* atomic operations in linux implicitly form a memory barrier */
#define membar_exit()
/*
* If a component has a reference to a kcf_provider_desc_t,
* it REFHOLD()s. A new provider descriptor which is referenced only
* by the providers table has a reference counter of one.
*/
#define KCF_PROV_REFHOLD(desc) { \
atomic_add_32(&(desc)->pd_refcnt, 1); \
ASSERT((desc)->pd_refcnt != 0); \
#define KCF_PROV_REFHOLD(desc) { \
int newval = atomic_add_32_nv(&(desc)->pd_refcnt, 1); \
ASSERT(newval != 0); \
}
#define KCF_PROV_IREFHOLD(desc) { \
atomic_add_32(&(desc)->pd_irefcnt, 1); \
ASSERT((desc)->pd_irefcnt != 0); \
#define KCF_PROV_IREFHOLD(desc) { \
int newval = atomic_add_32_nv(&(desc)->pd_irefcnt, 1); \
ASSERT(newval != 0); \
}
#define KCF_PROV_IREFRELE(desc) { \
ASSERT((desc)->pd_irefcnt != 0); \
membar_exit(); \
if (atomic_add_32_nv(&(desc)->pd_irefcnt, -1) == 0) { \
membar_producer(); \
int newval = atomic_add_32_nv(&(desc)->pd_irefcnt, -1); \
ASSERT(newval != -1); \
if (newval == 0) { \
cv_broadcast(&(desc)->pd_remove_cv); \
} \
}
@ -155,9 +153,10 @@ typedef struct kcf_provider_desc {
#define KCF_PROV_REFHELD(desc) ((desc)->pd_refcnt >= 1)
#define KCF_PROV_REFRELE(desc) { \
ASSERT((desc)->pd_refcnt != 0); \
membar_exit(); \
if (atomic_add_32_nv(&(desc)->pd_refcnt, -1) == 0) { \
membar_producer(); \
int newval = atomic_add_32_nv(&(desc)->pd_refcnt, -1); \
ASSERT(newval != -1); \
if (newval == 0) { \
kcf_provider_zero_refcnt((desc)); \
} \
}
@ -193,9 +192,9 @@ typedef struct kcf_mech_entry {
* it REFHOLD()s. A new policy descriptor which is referenced only
* by the policy table has a reference count of one.
*/
#define KCF_POLICY_REFHOLD(desc) { \
atomic_add_32(&(desc)->pd_refcnt, 1); \
ASSERT((desc)->pd_refcnt != 0); \
#define KCF_POLICY_REFHOLD(desc) { \
int newval = atomic_add_32_nv(&(desc)->pd_refcnt, 1); \
ASSERT(newval != 0); \
}
/*
@ -203,9 +202,10 @@ typedef struct kcf_mech_entry {
* reference is released, the descriptor is freed.
*/
#define KCF_POLICY_REFRELE(desc) { \
ASSERT((desc)->pd_refcnt != 0); \
membar_exit(); \
if (atomic_add_32_nv(&(desc)->pd_refcnt, -1) == 0) \
membar_producer(); \
int newval = atomic_add_32_nv(&(desc)->pd_refcnt, -1); \
ASSERT(newval != -1); \
if (newval == 0) \
kcf_policy_free_desc(desc); \
}

7
module/icp/include/sys/crypto/sched_impl.h
View File

@ -73,9 +73,10 @@ typedef struct kcf_context {
* context structure is freed along with the global context.
*/
#define KCF_CONTEXT_REFRELE(ictx) { \
ASSERT((ictx)->kc_refcnt != 0); \
membar_exit(); \
if (atomic_add_32_nv(&(ictx)->kc_refcnt, -1) == 0) \
membar_producer(); \
int newval = atomic_add_32_nv(&(ictx)->kc_refcnt, -1); \
ASSERT(newval != -1); \
if (newval == 0) \
kcf_free_context(ictx); \
}