On Mon, Jul 15, 2024 at 4:00 AM <[email protected]> wrote: > > > [PATCH] livepatch: support of modifying refcount_t without underflow after > unpatch > > CVE fixes sometimes add refcount_inc/dec() pairs to the code with existing > refcount_t. > Two problems arise when applying live-patch in this case: > 1) After refcount_t is being inc() during system is live-patched, after > unpatch the counter value will not be valid, as corresponing dec() would > never be called. > 2) Underflows are possible in runtime in case dec() is called before > corresponding inc() in the live-patched code. > > Proposed kprefcount_t functions are using following approach to solve these > two problems: > 1) In addition to original refcount_t, temporary refcount_t is allocated, and > after unpatch it is just removed. This way system is safe with correct > refcounting while patch is applied, and no underflow would happend after > unpatch. > 2) For inc/dec() added by live-patch code, one bit in reference-holder > structure is used (unsigned char *ref_holder, kprefholder_flag). In case > dec() is called first, it is just ignored as ref_holder bit would still not > be initialized. > > > API is defined include/linux/livepatch_refcount.h: > > typedef struct kprefcount_struct { > refcount_t *refcount; > refcount_t kprefcount; > spinlock_t lock; > } kprefcount_t; > > kprefcount_t *kprefcount_alloc(refcount_t *refcount, gfp_t flags); > void kprefcount_free(kprefcount_t *kp_ref); > int kprefcount_read(kprefcount_t *kp_ref); > void kprefcount_inc(kprefcount_t *kp_ref, unsigned char *ref_holder, int > kprefholder_flag); > void kprefcount_dec(kprefcount_t *kp_ref, unsigned char *ref_holder, int > kprefholder_flag); > bool kprefcount_dec_and_test(kprefcount_t *kp_ref, unsigned char *ref_holder, > int kprefholder_flag);
IIUC, kprefcount alone is not enough to solve the two issues. We still need some mechanism to manage the "ref_holder". Shadow variable is probably the best option here. The primary idea here is to enhance the refcount with a map. This may be too expensive in term memory consumption in some use cases. Overall, I don't think this change adds much more value on top of shadow variable. Thanks, Song
