From nobody Wed Apr 8 14:05:22 2026 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id E25EAC433FE for ; Wed, 26 Oct 2022 19:41:56 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234995AbiJZTlz (ORCPT ); Wed, 26 Oct 2022 15:41:55 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37656 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235013AbiJZTlj (ORCPT ); Wed, 26 Oct 2022 15:41:39 -0400 Received: from smtp-out2.suse.de (smtp-out2.suse.de [IPv6:2001:67c:2178:6::1d]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id EE142B978C; Wed, 26 Oct 2022 12:41:38 -0700 (PDT) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id 88FCB1FD6C; Wed, 26 Oct 2022 19:41:37 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1666813297; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=R2ZOrYe1VpBXYn04M+t8OPuDPgxBa02cJDnMZ+dKsEs=; b=M+kae/Jv/pCAjxtC8WxpkQ7bU1KNk/44f4tcVMgTefmZqA9KLxU89klgYcqoODkK0qtjpz jSLjSNjXCD+NmJ9kG0u5FVpzDouhF1ubeXUOq0hfL7ZCaMy9+bVOX+ETHzEl7V02sJlh6/ Dsj8wD1CE5ntP82y46AqOvMi80p2mMo= Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id AC3E713A77; Wed, 26 Oct 2022 19:41:35 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id oFkcHG+NWWNlFwAAMHmgww (envelope-from ); Wed, 26 Oct 2022 19:41:35 +0000 From: Marcos Paulo de Souza To: linux-kernel@vger.kernel.org, live-patching@vger.kernel.org Cc: jpoimboe@redhat.com, joe.lawrence@redhat.com, pmladek@suse.com, Marcos Paulo de Souza Subject: [PATCH v2 1/4] livepatch/shadow: Separate code to get or use pre-allocated shadow variable Date: Wed, 26 Oct 2022 16:41:19 -0300 Message-Id: <20221026194122.11761-2-mpdesouza@suse.com> X-Mailer: git-send-email 2.37.3 In-Reply-To: <20221026194122.11761-1-mpdesouza@suse.com> References: <20221026194122.11761-1-mpdesouza@suse.com> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Type: text/plain; charset="utf-8" From: Petr Mladek Separate code that is used in klp_shadow_get_or_alloc() under klp_mutex. It splits a long spaghetti function into two. Also it unifies the error handling. The old used a mix of duplicated code, direct returns, and goto. The new code has only one unlock, free, and return calls. It is code refactoring without any functional changes. Signed-off-by: Petr Mladek Signed-off-by: Marcos Paulo de Souza Reviewed-by: Petr Mladek --- Changes from v1: * Change __klp_shadow_get_or_use to __klp_shadow_get_or_add_locked and add= a comment about it's meaning (Petr) * Add lockdep_assert_held on __klp_shadow_get_or_add_locked * Reworked the commit message (Petr) * Added my SoB (Josh) kernel/livepatch/shadow.c | 78 ++++++++++++++++++++++----------------- 1 file changed, 45 insertions(+), 33 deletions(-) diff --git a/kernel/livepatch/shadow.c b/kernel/livepatch/shadow.c index c2e724d97ddf..81ad7cbbd124 100644 --- a/kernel/livepatch/shadow.c +++ b/kernel/livepatch/shadow.c @@ -101,41 +101,22 @@ void *klp_shadow_get(void *obj, unsigned long id) } EXPORT_SYMBOL_GPL(klp_shadow_get); =20 -static void *__klp_shadow_get_or_alloc(void *obj, unsigned long id, - size_t size, gfp_t gfp_flags, - klp_shadow_ctor_t ctor, void *ctor_data, - bool warn_on_exist) +/* Check if the variable exists. Otherwise, add the pre-allocated one. */ +static void *__klp_shadow_get_or_add_locked(void *obj, unsigned long id, + struct klp_shadow *new_shadow, + klp_shadow_ctor_t ctor, void *ctor_data, + bool *exist) { - struct klp_shadow *new_shadow; void *shadow_data; - unsigned long flags; =20 - /* Check if the shadow variable already exists */ - shadow_data =3D klp_shadow_get(obj, id); - if (shadow_data) - goto exists; - - /* - * Allocate a new shadow variable. Fill it with zeroes by default. - * More complex setting can be done by @ctor function. But it is - * called only when the buffer is really used (under klp_shadow_lock). - */ - new_shadow =3D kzalloc(size + sizeof(*new_shadow), gfp_flags); - if (!new_shadow) - return NULL; + lockdep_assert_held(&klp_shadow_lock); =20 - /* Look for again under the lock */ - spin_lock_irqsave(&klp_shadow_lock, flags); shadow_data =3D klp_shadow_get(obj, id); if (unlikely(shadow_data)) { - /* - * Shadow variable was found, throw away speculative - * allocation. - */ - spin_unlock_irqrestore(&klp_shadow_lock, flags); - kfree(new_shadow); - goto exists; + *exist =3D true; + return shadow_data; } + *exist =3D false; =20 new_shadow->obj =3D obj; new_shadow->id =3D id; @@ -145,8 +126,6 @@ static void *__klp_shadow_get_or_alloc(void *obj, unsig= ned long id, =20 err =3D ctor(obj, new_shadow->data, ctor_data); if (err) { - spin_unlock_irqrestore(&klp_shadow_lock, flags); - kfree(new_shadow); pr_err("Failed to construct shadow variable <%p, %lx> (%d)\n", obj, id, err); return NULL; @@ -156,12 +135,45 @@ static void *__klp_shadow_get_or_alloc(void *obj, uns= igned long id, /* No found, so attach the newly allocated one */ hash_add_rcu(klp_shadow_hash, &new_shadow->node, (unsigned long)new_shadow->obj); - spin_unlock_irqrestore(&klp_shadow_lock, flags); =20 return new_shadow->data; +} + +static void *__klp_shadow_get_or_alloc(void *obj, unsigned long id, + size_t size, gfp_t gfp_flags, + klp_shadow_ctor_t ctor, void *ctor_data, + bool warn_on_exist) +{ + struct klp_shadow *new_shadow; + void *shadow_data; + bool exist; + unsigned long flags; + + /* Check if the shadow variable already exists */ + shadow_data =3D klp_shadow_get(obj, id); + if (shadow_data) + return shadow_data; + + /* + * Allocate a new shadow variable. Fill it with zeroes by default. + * More complex setting can be done by @ctor function. But it is + * called only when the buffer is really used (under klp_shadow_lock). + */ + new_shadow =3D kzalloc(size + sizeof(*new_shadow), gfp_flags); + if (!new_shadow) + return NULL; + + /* Look for again under the lock */ + spin_lock_irqsave(&klp_shadow_lock, flags); + shadow_data =3D __klp_shadow_get_or_add_locked(obj, id, new_shadow, + ctor, ctor_data, &exist); + spin_unlock_irqrestore(&klp_shadow_lock, flags); + + /* Throw away unused speculative allocation. */ + if (!shadow_data || exist) + kfree(new_shadow); =20 -exists: - if (warn_on_exist) { + if (exist && warn_on_exist) { WARN(1, "Duplicate shadow variable <%p, %lx>\n", obj, id); return NULL; } --=20 2.37.3 From nobody Wed Apr 8 14:05:22 2026 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 52142C38A2D for ; Wed, 26 Oct 2022 19:41:59 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234994AbiJZTl5 (ORCPT ); Wed, 26 Oct 2022 15:41:57 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:38562 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234997AbiJZTlp (ORCPT ); Wed, 26 Oct 2022 15:41:45 -0400 Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 9096CB4899; Wed, 26 Oct 2022 12:41:41 -0700 (PDT) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id 51F781FD70; Wed, 26 Oct 2022 19:41:40 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1666813300; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=XaNif8Cw4uustU+ovZVF6G4VJf04B2ti3O7xS7XvquQ=; b=SvwwGe9CaAHchGBB+YPr9VxpoHxmlfnVr7TipKs3dk5HkX9Grue8h8HlRthHi/TW7TYiKP UMHcYfYadKHctOjGG4CHuqH/C7yx+5SDF233K7Z9gpkK0vC/n4wJJSg1LMCu70Xb0iKG5e nzMxl+aZfgxraGQKjndTht2m+Rh78Ek= Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 0004013A77; Wed, 26 Oct 2022 19:41:37 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id CLAALnGNWWNlFwAAMHmgww (envelope-from ); Wed, 26 Oct 2022 19:41:37 +0000 From: Marcos Paulo de Souza To: linux-kernel@vger.kernel.org, live-patching@vger.kernel.org Cc: jpoimboe@redhat.com, joe.lawrence@redhat.com, pmladek@suse.com, Marcos Paulo de Souza Subject: [PATCH v2 2/4] livepatch/shadow: Separate code removing all shadow variables for a given id Date: Wed, 26 Oct 2022 16:41:20 -0300 Message-Id: <20221026194122.11761-3-mpdesouza@suse.com> X-Mailer: git-send-email 2.37.3 In-Reply-To: <20221026194122.11761-1-mpdesouza@suse.com> References: <20221026194122.11761-1-mpdesouza@suse.com> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Type: text/plain; charset="utf-8" From: Petr Mladek Allow to remove all shadow variables with already taken klp_shadow_lock. It will be needed to synchronize this with other operation during the garbage collection of shadow variables. It is a code refactoring without any functional changes. Signed-off-by: Petr Mladek Reviewed-by: Petr Mladek Signed-off-by: Marcos Paulo de Souza --- Changes from v1: * Added my SoB (Josh) kernel/livepatch/shadow.c | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/kernel/livepatch/shadow.c b/kernel/livepatch/shadow.c index 81ad7cbbd124..aba44dcc0a88 100644 --- a/kernel/livepatch/shadow.c +++ b/kernel/livepatch/shadow.c @@ -283,6 +283,20 @@ void klp_shadow_free(void *obj, unsigned long id, klp_= shadow_dtor_t dtor) } EXPORT_SYMBOL_GPL(klp_shadow_free); =20 +static void __klp_shadow_free_all(unsigned long id, klp_shadow_dtor_t dtor) +{ + struct klp_shadow *shadow; + int i; + + lockdep_assert_held(&klp_shadow_lock); + + /* Delete all <*, id> from hash */ + hash_for_each(klp_shadow_hash, i, shadow, node) { + if (klp_shadow_match(shadow, shadow->obj, id)) + klp_shadow_free_struct(shadow, dtor); + } +} + /** * klp_shadow_free_all() - detach and free all <_, id> shadow variables * @id: data identifier @@ -294,18 +308,10 @@ EXPORT_SYMBOL_GPL(klp_shadow_free); */ void klp_shadow_free_all(unsigned long id, klp_shadow_dtor_t dtor) { - struct klp_shadow *shadow; unsigned long flags; - int i; =20 spin_lock_irqsave(&klp_shadow_lock, flags); - - /* Delete all <_, id> from hash */ - hash_for_each(klp_shadow_hash, i, shadow, node) { - if (klp_shadow_match(shadow, shadow->obj, id)) - klp_shadow_free_struct(shadow, dtor); - } - + __klp_shadow_free_all(id, dtor); spin_unlock_irqrestore(&klp_shadow_lock, flags); } EXPORT_SYMBOL_GPL(klp_shadow_free_all); --=20 2.37.3 From nobody Wed Apr 8 14:05:22 2026 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id A87C2C433FE for ; Wed, 26 Oct 2022 19:42:05 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235033AbiJZTmC (ORCPT ); Wed, 26 Oct 2022 15:42:02 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37488 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234259AbiJZTlr (ORCPT ); Wed, 26 Oct 2022 15:41:47 -0400 Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 90BF9B7F60; Wed, 26 Oct 2022 12:41:43 -0700 (PDT) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id A3FCC21FFC; Wed, 26 Oct 2022 19:41:42 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1666813302; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Gei3z6v/UM+glPIhIg6yXmE+bgYZJKhOzcfEW7UYnXE=; b=N2iFmJwA+h3N3On0xvFXRRkwYaywMAhOASQebk5eH9uB1w8BuhnS3NoPBCeCkTQU5JZ5Hy 5yPxXIEMO0NHC4SvR0DVLwsSQT0nfW9NXZln5BULDOWkdEWGmps/VC7k2guLQwucYOVK5R 3qozWqfYAWAzWjP34pSHjF9EYOY9wjo= Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id B93CB13A77; Wed, 26 Oct 2022 19:41:40 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id GFXDH3SNWWNlFwAAMHmgww (envelope-from ); Wed, 26 Oct 2022 19:41:40 +0000 From: Marcos Paulo de Souza To: linux-kernel@vger.kernel.org, live-patching@vger.kernel.org Cc: jpoimboe@redhat.com, joe.lawrence@redhat.com, pmladek@suse.com, Marcos Paulo de Souza Subject: [PATCH v2 3/4] livepatch/shadow: Introduce klp_shadow_type structure Date: Wed, 26 Oct 2022 16:41:21 -0300 Message-Id: <20221026194122.11761-4-mpdesouza@suse.com> X-Mailer: git-send-email 2.37.3 In-Reply-To: <20221026194122.11761-1-mpdesouza@suse.com> References: <20221026194122.11761-1-mpdesouza@suse.com> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Type: text/plain; charset="utf-8" The shadow variable type will be used in klp_shadow_alloc/get/free functions instead of id/ctor/dtor parameters. As a result, all callers use the same callbacks consistently[*][**]. The structure will be used in the next patch that will manage the lifetime of shadow variables and execute garbage collection automatically. [*] From the user POV, it might have been easier to pass $id instead of pointer to struct klp_shadow_type. It would require registering the klp_shadow_type so that the klp_shadow API could find ctor/dtor for the given id. It actually will be needed for the garbage collection anyway because it will define the lifetime of the variables. The bigger problem is that the same klp_shadow_type might be used by more livepatch modules. Each livepatch module need to duplicate the definition of klp_shadow_type and ctor/dtor callbacks. The klp_shadow API would need to choose one registered copy. The definitions should be compatible and they should stay as long as the type is registered. But it still feels more safe when klp_shadow API callers use struct klp_shadow_type and ctor/dtor callbacks defined in the same livepatch module. This problem is gone when each livepatch explicitly uses its own struct klp_shadow_type pointing to its own callbacks. [**] test_klp_shadow_vars.c uses a custom @dtor to show that it was called. The message must be disabled when called via klp_shadow_free_all() because the ordering of freed variables is not well defined there. It has to be done using another hack after switching to klp_shadow_types. Co-developed-by: Petr Mladek Signed-off-by: Petr Mladek Signed-off-by: Marcos Paulo de Souza Reviewed-by: Petr Mladek --- Changes from v1: * Added my SoB (Josh) * Added a Co-developed-by tag (Petr) * Changed the comment about throwing away speculative allocation (Petr) include/linux/livepatch.h | 29 +++-- kernel/livepatch/shadow.c | 107 +++++++++--------- lib/livepatch/test_klp_shadow_vars.c | 105 ++++++++++------- samples/livepatch/livepatch-shadow-fix1.c | 18 ++- samples/livepatch/livepatch-shadow-fix2.c | 27 +++-- .../selftests/livepatch/test-shadow-vars.sh | 2 +- 6 files changed, 165 insertions(+), 123 deletions(-) diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h index 293e29960c6e..79e7bf3b35f6 100644 --- a/include/linux/livepatch.h +++ b/include/linux/livepatch.h @@ -216,15 +216,26 @@ typedef int (*klp_shadow_ctor_t)(void *obj, void *ctor_data); typedef void (*klp_shadow_dtor_t)(void *obj, void *shadow_data); =20 -void *klp_shadow_get(void *obj, unsigned long id); -void *klp_shadow_alloc(void *obj, unsigned long id, - size_t size, gfp_t gfp_flags, - klp_shadow_ctor_t ctor, void *ctor_data); -void *klp_shadow_get_or_alloc(void *obj, unsigned long id, - size_t size, gfp_t gfp_flags, - klp_shadow_ctor_t ctor, void *ctor_data); -void klp_shadow_free(void *obj, unsigned long id, klp_shadow_dtor_t dtor); -void klp_shadow_free_all(unsigned long id, klp_shadow_dtor_t dtor); +/** + * struct klp_shadow_type - shadow variable type used by the klp_object + * @id: shadow variable type indentifier + * @ctor: custom constructor to initialize the shadow data (optional) + * @dtor: custom callback that can be used to unregister the variable + * and/or free data that the shadow variable points to (optional) + */ +struct klp_shadow_type { + unsigned long id; + klp_shadow_ctor_t ctor; + klp_shadow_dtor_t dtor; +}; + +void *klp_shadow_get(void *obj, struct klp_shadow_type *shadow_type); +void *klp_shadow_alloc(void *obj, struct klp_shadow_type *shadow_type, + size_t size, gfp_t gfp_flags, void *ctor_data); +void *klp_shadow_get_or_alloc(void *obj, struct klp_shadow_type *shadow_ty= pe, + size_t size, gfp_t gfp_flags, void *ctor_data); +void klp_shadow_free(void *obj, struct klp_shadow_type *shadow_type); +void klp_shadow_free_all(struct klp_shadow_type *shadow_type); =20 struct klp_state *klp_get_state(struct klp_patch *patch, unsigned long id); struct klp_state *klp_get_prev_state(unsigned long id); diff --git a/kernel/livepatch/shadow.c b/kernel/livepatch/shadow.c index aba44dcc0a88..64e83853891d 100644 --- a/kernel/livepatch/shadow.c +++ b/kernel/livepatch/shadow.c @@ -63,24 +63,24 @@ struct klp_shadow { * klp_shadow_match() - verify a shadow variable matches given * @shadow: shadow variable to match * @obj: pointer to parent object - * @id: data identifier + * @shadow_type: type of the wanted shadow variable * * Return: true if the shadow variable matches. */ static inline bool klp_shadow_match(struct klp_shadow *shadow, void *obj, - unsigned long id) + struct klp_shadow_type *shadow_type) { - return shadow->obj =3D=3D obj && shadow->id =3D=3D id; + return shadow->obj =3D=3D obj && shadow->id =3D=3D shadow_type->id; } =20 /** * klp_shadow_get() - retrieve a shadow variable data pointer * @obj: pointer to parent object - * @id: data identifier + * @shadow_type: type of the wanted shadow variable * * Return: the shadow variable data element, NULL on failure. */ -void *klp_shadow_get(void *obj, unsigned long id) +void *klp_shadow_get(void *obj, struct klp_shadow_type *shadow_type) { struct klp_shadow *shadow; =20 @@ -89,7 +89,7 @@ void *klp_shadow_get(void *obj, unsigned long id) hash_for_each_possible_rcu(klp_shadow_hash, shadow, node, (unsigned long)obj) { =20 - if (klp_shadow_match(shadow, obj, id)) { + if (klp_shadow_match(shadow, obj, shadow_type)) { rcu_read_unlock(); return shadow->data; } @@ -101,17 +101,16 @@ void *klp_shadow_get(void *obj, unsigned long id) } EXPORT_SYMBOL_GPL(klp_shadow_get); =20 -/* Check if the variable exists. Otherwise, add the pre-allocated one. */ -static void *__klp_shadow_get_or_add_locked(void *obj, unsigned long id, - struct klp_shadow *new_shadow, - klp_shadow_ctor_t ctor, void *ctor_data, - bool *exist) +static void *__klp_shadow_get_or_add_locked(void *obj, + struct klp_shadow_type *shadow_type, + struct klp_shadow *new_shadow, + void *ctor_data, bool *exist) { void *shadow_data; =20 lockdep_assert_held(&klp_shadow_lock); =20 - shadow_data =3D klp_shadow_get(obj, id); + shadow_data =3D klp_shadow_get(obj, shadow_type); if (unlikely(shadow_data)) { *exist =3D true; return shadow_data; @@ -119,15 +118,15 @@ static void *__klp_shadow_get_or_add_locked(void *obj= , unsigned long id, *exist =3D false; =20 new_shadow->obj =3D obj; - new_shadow->id =3D id; + new_shadow->id =3D shadow_type->id; =20 - if (ctor) { + if (shadow_type->ctor) { int err; =20 - err =3D ctor(obj, new_shadow->data, ctor_data); + err =3D shadow_type->ctor(obj, new_shadow->data, ctor_data); if (err) { pr_err("Failed to construct shadow variable <%p, %lx> (%d)\n", - obj, id, err); + obj, shadow_type->id, err); return NULL; } } @@ -139,9 +138,8 @@ static void *__klp_shadow_get_or_add_locked(void *obj, = unsigned long id, return new_shadow->data; } =20 -static void *__klp_shadow_get_or_alloc(void *obj, unsigned long id, - size_t size, gfp_t gfp_flags, - klp_shadow_ctor_t ctor, void *ctor_data, +static void *__klp_shadow_get_or_alloc(void *obj, struct klp_shadow_type *= shadow_type, + size_t size, gfp_t gfp_flags, void *ctor_data, bool warn_on_exist) { struct klp_shadow *new_shadow; @@ -150,7 +148,7 @@ static void *__klp_shadow_get_or_alloc(void *obj, unsig= ned long id, unsigned long flags; =20 /* Check if the shadow variable already exists */ - shadow_data =3D klp_shadow_get(obj, id); + shadow_data =3D klp_shadow_get(obj, shadow_type); if (shadow_data) return shadow_data; =20 @@ -159,22 +157,25 @@ static void *__klp_shadow_get_or_alloc(void *obj, uns= igned long id, * More complex setting can be done by @ctor function. But it is * called only when the buffer is really used (under klp_shadow_lock). */ - new_shadow =3D kzalloc(size + sizeof(*new_shadow), gfp_flags); + new_shadow =3D kzalloc(size + sizeof(struct klp_shadow), gfp_flags); if (!new_shadow) return NULL; =20 /* Look for again under the lock */ spin_lock_irqsave(&klp_shadow_lock, flags); - shadow_data =3D __klp_shadow_get_or_add_locked(obj, id, new_shadow, - ctor, ctor_data, &exist); + shadow_data =3D __klp_shadow_get_or_add_locked(obj, shadow_type, + new_shadow, ctor_data, &exist); spin_unlock_irqrestore(&klp_shadow_lock, flags); =20 - /* Throw away unused speculative allocation. */ + /* + * Throw away unused speculative allocation if the ctor() failed + * or the variable already existed. + */ if (!shadow_data || exist) kfree(new_shadow); =20 if (exist && warn_on_exist) { - WARN(1, "Duplicate shadow variable <%p, %lx>\n", obj, id); + WARN(1, "Duplicate shadow variable <%p, %lx>\n", obj, shadow_type->id); return NULL; } =20 @@ -184,10 +185,9 @@ static void *__klp_shadow_get_or_alloc(void *obj, unsi= gned long id, /** * klp_shadow_alloc() - allocate and add a new shadow variable * @obj: pointer to parent object - * @id: data identifier + * @shadow_type: type of the wanted shadow variable * @size: size of attached data * @gfp_flags: GFP mask for allocation - * @ctor: custom constructor to initialize the shadow data (optional) * @ctor_data: pointer to any data needed by @ctor (optional) * * Allocates @size bytes for new shadow variable data using @gfp_flags. @@ -205,22 +205,21 @@ static void *__klp_shadow_get_or_alloc(void *obj, uns= igned long id, * Return: the shadow variable data element, NULL on duplicate or * failure. */ -void *klp_shadow_alloc(void *obj, unsigned long id, - size_t size, gfp_t gfp_flags, - klp_shadow_ctor_t ctor, void *ctor_data) +void *klp_shadow_alloc(void *obj, struct klp_shadow_type *shadow_type, + size_t size, gfp_t gfp_flags, void *ctor_data) { - return __klp_shadow_get_or_alloc(obj, id, size, gfp_flags, - ctor, ctor_data, true); + return __klp_shadow_get_or_alloc(obj, shadow_type, size, + gfp_flags, ctor_data, + true); } EXPORT_SYMBOL_GPL(klp_shadow_alloc); =20 /** * klp_shadow_get_or_alloc() - get existing or allocate a new shadow varia= ble * @obj: pointer to parent object - * @id: data identifier + * @shadow_type: type of the wanted shadow variable * @size: size of attached data * @gfp_flags: GFP mask for allocation - * @ctor: custom constructor to initialize the shadow data (optional) * @ctor_data: pointer to any data needed by @ctor (optional) * * Returns a pointer to existing shadow data if an shadow @@ -234,35 +233,33 @@ EXPORT_SYMBOL_GPL(klp_shadow_alloc); * * Return: the shadow variable data element, NULL on failure. */ -void *klp_shadow_get_or_alloc(void *obj, unsigned long id, - size_t size, gfp_t gfp_flags, - klp_shadow_ctor_t ctor, void *ctor_data) +void *klp_shadow_get_or_alloc(void *obj, struct klp_shadow_type *shadow_ty= pe, + size_t size, gfp_t gfp_flags, void *ctor_data) { - return __klp_shadow_get_or_alloc(obj, id, size, gfp_flags, - ctor, ctor_data, false); + return __klp_shadow_get_or_alloc(obj, shadow_type, size, + gfp_flags, ctor_data, + false); } EXPORT_SYMBOL_GPL(klp_shadow_get_or_alloc); =20 static void klp_shadow_free_struct(struct klp_shadow *shadow, - klp_shadow_dtor_t dtor) + struct klp_shadow_type *shadow_type) { hash_del_rcu(&shadow->node); - if (dtor) - dtor(shadow->obj, shadow->data); + if (shadow_type->dtor) + shadow_type->dtor(shadow->obj, shadow->data); kfree_rcu(shadow, rcu_head); } =20 /** * klp_shadow_free() - detach and free a shadow variable * @obj: pointer to parent object - * @id: data identifier - * @dtor: custom callback that can be used to unregister the variable - * and/or free data that the shadow variable points to (optional) + * @shadow_type: type of to be freed shadow variable * * This function releases the memory for this shadow variable * instance, callers should stop referencing it accordingly. */ -void klp_shadow_free(void *obj, unsigned long id, klp_shadow_dtor_t dtor) +void klp_shadow_free(void *obj, struct klp_shadow_type *shadow_type) { struct klp_shadow *shadow; unsigned long flags; @@ -273,8 +270,8 @@ void klp_shadow_free(void *obj, unsigned long id, klp_s= hadow_dtor_t dtor) hash_for_each_possible(klp_shadow_hash, shadow, node, (unsigned long)obj) { =20 - if (klp_shadow_match(shadow, obj, id)) { - klp_shadow_free_struct(shadow, dtor); + if (klp_shadow_match(shadow, obj, shadow_type)) { + klp_shadow_free_struct(shadow, shadow_type); break; } } @@ -283,7 +280,7 @@ void klp_shadow_free(void *obj, unsigned long id, klp_s= hadow_dtor_t dtor) } EXPORT_SYMBOL_GPL(klp_shadow_free); =20 -static void __klp_shadow_free_all(unsigned long id, klp_shadow_dtor_t dtor) +static void __klp_shadow_free_all(struct klp_shadow_type *shadow_type) { struct klp_shadow *shadow; int i; @@ -292,26 +289,24 @@ static void __klp_shadow_free_all(unsigned long id, k= lp_shadow_dtor_t dtor) =20 /* Delete all <*, id> from hash */ hash_for_each(klp_shadow_hash, i, shadow, node) { - if (klp_shadow_match(shadow, shadow->obj, id)) - klp_shadow_free_struct(shadow, dtor); + if (klp_shadow_match(shadow, shadow->obj, shadow_type)) + klp_shadow_free_struct(shadow, shadow_type); } } =20 /** * klp_shadow_free_all() - detach and free all <_, id> shadow variables - * @id: data identifier - * @dtor: custom callback that can be used to unregister the variable - * and/or free data that the shadow variable points to (optional) + * @shadow_type: type of to be freed shadow variables * * This function releases the memory for all <_, id> shadow variable * instances, callers should stop referencing them accordingly. */ -void klp_shadow_free_all(unsigned long id, klp_shadow_dtor_t dtor) +void klp_shadow_free_all(struct klp_shadow_type *shadow_type) { unsigned long flags; =20 spin_lock_irqsave(&klp_shadow_lock, flags); - __klp_shadow_free_all(id, dtor); + __klp_shadow_free_all(shadow_type); spin_unlock_irqrestore(&klp_shadow_lock, flags); } EXPORT_SYMBOL_GPL(klp_shadow_free_all); diff --git a/lib/livepatch/test_klp_shadow_vars.c b/lib/livepatch/test_klp_= shadow_vars.c index b99116490858..ee47c1fae8e2 100644 --- a/lib/livepatch/test_klp_shadow_vars.c +++ b/lib/livepatch/test_klp_shadow_vars.c @@ -58,58 +58,64 @@ static int ptr_id(void *ptr) * to the kernel log for testing verification. Don't display raw pointers, * but use the ptr_id() value instead. */ -static void *shadow_get(void *obj, unsigned long id) +static void *shadow_get(void *obj, struct klp_shadow_type *shadow_type) { int **sv; =20 - sv =3D klp_shadow_get(obj, id); + sv =3D klp_shadow_get(obj, shadow_type); pr_info("klp_%s(obj=3DPTR%d, id=3D0x%lx) =3D PTR%d\n", - __func__, ptr_id(obj), id, ptr_id(sv)); + __func__, ptr_id(obj), shadow_type->id, ptr_id(sv)); =20 return sv; } =20 -static void *shadow_alloc(void *obj, unsigned long id, size_t size, - gfp_t gfp_flags, klp_shadow_ctor_t ctor, - void *ctor_data) +static void *shadow_alloc(void *obj, struct klp_shadow_type *shadow_type, + size_t size, gfp_t gfp_flags, void *ctor_data) { int **var =3D ctor_data; int **sv; =20 - sv =3D klp_shadow_alloc(obj, id, size, gfp_flags, ctor, var); + sv =3D klp_shadow_alloc(obj, shadow_type, size, gfp_flags, var); pr_info("klp_%s(obj=3DPTR%d, id=3D0x%lx, size=3D%zx, gfp_flags=3D%pGg), c= tor=3DPTR%d, ctor_data=3DPTR%d =3D PTR%d\n", - __func__, ptr_id(obj), id, size, &gfp_flags, ptr_id(ctor), + __func__, ptr_id(obj), shadow_type->id, size, &gfp_flags, ptr_id(shadow_= type->ctor), ptr_id(*var), ptr_id(sv)); =20 return sv; } =20 -static void *shadow_get_or_alloc(void *obj, unsigned long id, size_t size, - gfp_t gfp_flags, klp_shadow_ctor_t ctor, - void *ctor_data) +static void *shadow_get_or_alloc(void *obj, struct klp_shadow_type *shadow= _type, + size_t size, gfp_t gfp_flags, void *ctor_data) { int **var =3D ctor_data; int **sv; =20 - sv =3D klp_shadow_get_or_alloc(obj, id, size, gfp_flags, ctor, var); + sv =3D klp_shadow_get_or_alloc(obj, shadow_type, size, gfp_flags, var); pr_info("klp_%s(obj=3DPTR%d, id=3D0x%lx, size=3D%zx, gfp_flags=3D%pGg), c= tor=3DPTR%d, ctor_data=3DPTR%d =3D PTR%d\n", - __func__, ptr_id(obj), id, size, &gfp_flags, ptr_id(ctor), + __func__, ptr_id(obj), shadow_type->id, size, &gfp_flags, ptr_id(shadow_= type->ctor), ptr_id(*var), ptr_id(sv)); =20 return sv; } =20 -static void shadow_free(void *obj, unsigned long id, klp_shadow_dtor_t dto= r) +static void shadow_free(void *obj, struct klp_shadow_type *shadow_type) { - klp_shadow_free(obj, id, dtor); + klp_shadow_free(obj, shadow_type); pr_info("klp_%s(obj=3DPTR%d, id=3D0x%lx, dtor=3DPTR%d)\n", - __func__, ptr_id(obj), id, ptr_id(dtor)); + __func__, ptr_id(obj), shadow_type->id, ptr_id(shadow_type->dtor)); } =20 -static void shadow_free_all(unsigned long id, klp_shadow_dtor_t dtor) +/* + * With more than one item to free in the list, order is not determined and + * shadow_dtor will not be passed to shadow_free_all() which would make the + * test fail. (see pass 6) + */ +static bool verbose_dtor =3D true; +static void shadow_free_all(struct klp_shadow_type *shadow_type) { - klp_shadow_free_all(id, dtor); - pr_info("klp_%s(id=3D0x%lx, dtor=3DPTR%d)\n", __func__, id, ptr_id(dtor)); + verbose_dtor =3D false; + klp_shadow_free_all(shadow_type); + verbose_dtor =3D true; + pr_info("klp_%s(id=3D0x%lx, dtor=3DPTR%d)\n", __func__, shadow_type->id, = ptr_id(shadow_type->dtor)); } =20 =20 @@ -128,17 +134,14 @@ static int shadow_ctor(void *obj, void *shadow_data, = void *ctor_data) return 0; } =20 -/* - * With more than one item to free in the list, order is not determined and - * shadow_dtor will not be passed to shadow_free_all() which would make the - * test fail. (see pass 6) - */ static void shadow_dtor(void *obj, void *shadow_data) { int **sv =3D shadow_data; =20 - pr_info("%s(obj=3DPTR%d, shadow_data=3DPTR%d)\n", - __func__, ptr_id(obj), ptr_id(sv)); + if (verbose_dtor) { + pr_info("%s(obj=3DPTR%d, shadow_data=3DPTR%d)\n", + __func__, ptr_id(obj), ptr_id(sv)); + } } =20 /* number of objects we simulate that need shadow vars */ @@ -148,6 +151,18 @@ static void shadow_dtor(void *obj, void *shadow_data) #define SV_ID1 0x1234 #define SV_ID2 0x1235 =20 +struct klp_shadow_type shadow_type_1 =3D { + .id =3D SV_ID1, + .ctor =3D shadow_ctor, + .dtor =3D shadow_dtor, +}; + +struct klp_shadow_type shadow_type_2 =3D { + .id =3D SV_ID2, + .ctor =3D shadow_ctor, + .dtor =3D shadow_dtor, +}; + /* * The main test case adds/removes new fields (shadow var) to each of these * test structure instances. The last group of fields in the struct repres= ent @@ -179,7 +194,7 @@ static int test_klp_shadow_vars_init(void) * With an empty shadow variable hash table, expect not to find * any matches. */ - sv =3D shadow_get(&objs[0], SV_ID1); + sv =3D shadow_get(&objs[0], &shadow_type_1); if (!sv) pr_info(" got expected NULL result\n"); =20 @@ -189,13 +204,13 @@ static int test_klp_shadow_vars_init(void) ptr_id(pnfields1[i]); =20 if (i % 2) { - sv1[i] =3D shadow_alloc(&objs[i], SV_ID1, + sv1[i] =3D shadow_alloc(&objs[i], &shadow_type_1, sizeof(pnfields1[i]), GFP_KERNEL, - shadow_ctor, &pnfields1[i]); + &pnfields1[i]); } else { - sv1[i] =3D shadow_get_or_alloc(&objs[i], SV_ID1, + sv1[i] =3D shadow_get_or_alloc(&objs[i], &shadow_type_1, sizeof(pnfields1[i]), GFP_KERNEL, - shadow_ctor, &pnfields1[i]); + &pnfields1[i]); } if (!sv1[i]) { ret =3D -ENOMEM; @@ -204,8 +219,9 @@ static int test_klp_shadow_vars_init(void) =20 pnfields2[i] =3D &nfields2[i]; ptr_id(pnfields2[i]); - sv2[i] =3D shadow_alloc(&objs[i], SV_ID2, sizeof(pnfields2[i]), - GFP_KERNEL, shadow_ctor, &pnfields2[i]); + sv2[i] =3D shadow_alloc(&objs[i], &shadow_type_2, + sizeof(pnfields2[i]), + GFP_KERNEL, &pnfields2[i]); if (!sv2[i]) { ret =3D -ENOMEM; goto out; @@ -215,7 +231,7 @@ static int test_klp_shadow_vars_init(void) /* pass 2: verify we find allocated svars and where they point to */ for (i =3D 0; i < NUM_OBJS; i++) { /* check the "char" svar for all objects */ - sv =3D shadow_get(&objs[i], SV_ID1); + sv =3D shadow_get(&objs[i], &shadow_type_1); if (!sv) { ret =3D -EINVAL; goto out; @@ -225,7 +241,7 @@ static int test_klp_shadow_vars_init(void) ptr_id(sv1[i]), ptr_id(*sv1[i])); =20 /* check the "int" svar for all objects */ - sv =3D shadow_get(&objs[i], SV_ID2); + sv =3D shadow_get(&objs[i], &shadow_type_2); if (!sv) { ret =3D -EINVAL; goto out; @@ -240,8 +256,9 @@ static int test_klp_shadow_vars_init(void) pndup[i] =3D &nfields1[i]; ptr_id(pndup[i]); =20 - sv =3D shadow_get_or_alloc(&objs[i], SV_ID1, sizeof(pndup[i]), - GFP_KERNEL, shadow_ctor, &pndup[i]); + sv =3D shadow_get_or_alloc(&objs[i], &shadow_type_1, + sizeof(pndup[i]), + GFP_KERNEL, &pndup[i]); if (!sv) { ret =3D -EINVAL; goto out; @@ -253,15 +270,15 @@ static int test_klp_shadow_vars_init(void) =20 /* pass 4: free pairs of svars, verify removal */ for (i =3D 0; i < NUM_OBJS; i++) { - shadow_free(&objs[i], SV_ID1, shadow_dtor); /* 'char' pairs */ - sv =3D shadow_get(&objs[i], SV_ID1); + shadow_free(&objs[i], &shadow_type_1); /* 'char' pairs */ + sv =3D shadow_get(&objs[i], &shadow_type_1); if (!sv) pr_info(" got expected NULL result\n"); } =20 /* pass 5: check we still find svar pairs */ for (i =3D 0; i < NUM_OBJS; i++) { - sv =3D shadow_get(&objs[i], SV_ID2); /* 'int' pairs */ + sv =3D shadow_get(&objs[i], &shadow_type_2); /* 'int' pairs */ if (!sv) { ret =3D -EINVAL; goto out; @@ -272,9 +289,9 @@ static int test_klp_shadow_vars_init(void) } =20 /* pass 6: free all the svar pairs too. */ - shadow_free_all(SV_ID2, NULL); /* 'int' pairs */ + shadow_free_all(&shadow_type_2); /* 'int' pairs */ for (i =3D 0; i < NUM_OBJS; i++) { - sv =3D shadow_get(&objs[i], SV_ID2); + sv =3D shadow_get(&objs[i], &shadow_type_2); if (!sv) pr_info(" got expected NULL result\n"); } @@ -283,8 +300,8 @@ static int test_klp_shadow_vars_init(void) =20 return 0; out: - shadow_free_all(SV_ID1, NULL); /* 'char' pairs */ - shadow_free_all(SV_ID2, NULL); /* 'int' pairs */ + shadow_free_all(&shadow_type_1); /* 'char' pairs */ + shadow_free_all(&shadow_type_2); /* 'int' pairs */ free_ptr_list(); =20 return ret; diff --git a/samples/livepatch/livepatch-shadow-fix1.c b/samples/livepatch/= livepatch-shadow-fix1.c index 6701641bf12d..0cc7d1e4b4bc 100644 --- a/samples/livepatch/livepatch-shadow-fix1.c +++ b/samples/livepatch/livepatch-shadow-fix1.c @@ -32,6 +32,8 @@ /* Shadow variable enums */ #define SV_LEAK 1 =20 +static struct klp_shadow_type shadow_leak_type; + /* Allocate new dummies every second */ #define ALLOC_PERIOD 1 /* Check for expired dummies after a few new ones have been allocated */ @@ -84,8 +86,8 @@ static struct dummy *livepatch_fix1_dummy_alloc(void) if (!leak) goto err_leak; =20 - shadow_leak =3D klp_shadow_alloc(d, SV_LEAK, sizeof(leak), GFP_KERNEL, - shadow_leak_ctor, &leak); + shadow_leak =3D klp_shadow_alloc(d, &shadow_leak_type, sizeof(leak), + GFP_KERNEL, &leak); if (!shadow_leak) { pr_err("%s: failed to allocate shadow variable for the leaking pointer: = dummy @ %p, leak @ %p\n", __func__, d, leak); @@ -124,15 +126,21 @@ static void livepatch_fix1_dummy_free(struct dummy *d) * not exist (ie, dummy structures allocated before this livepatch * was loaded.) */ - shadow_leak =3D klp_shadow_get(d, SV_LEAK); + shadow_leak =3D klp_shadow_get(d, &shadow_leak_type); if (shadow_leak) - klp_shadow_free(d, SV_LEAK, livepatch_fix1_dummy_leak_dtor); + klp_shadow_free(d, &shadow_leak_type); else pr_info("%s: dummy @ %p leaked!\n", __func__, d); =20 kfree(d); } =20 +static struct klp_shadow_type shadow_leak_type =3D { + .id =3D SV_LEAK, + .ctor =3D shadow_leak_ctor, + .dtor =3D livepatch_fix1_dummy_leak_dtor, +}; + static struct klp_func funcs[] =3D { { .old_name =3D "dummy_alloc", @@ -164,7 +172,7 @@ static int livepatch_shadow_fix1_init(void) static void livepatch_shadow_fix1_exit(void) { /* Cleanup any existing SV_LEAK shadow variables */ - klp_shadow_free_all(SV_LEAK, livepatch_fix1_dummy_leak_dtor); + klp_shadow_free_all(&shadow_leak_type); } =20 module_init(livepatch_shadow_fix1_init); diff --git a/samples/livepatch/livepatch-shadow-fix2.c b/samples/livepatch/= livepatch-shadow-fix2.c index 361046a4f10c..840100555152 100644 --- a/samples/livepatch/livepatch-shadow-fix2.c +++ b/samples/livepatch/livepatch-shadow-fix2.c @@ -33,6 +33,9 @@ #define SV_LEAK 1 #define SV_COUNTER 2 =20 +static struct klp_shadow_type shadow_leak_type; +static struct klp_shadow_type shadow_counter_type; + struct dummy { struct list_head list; unsigned long jiffies_expire; @@ -47,9 +50,8 @@ static bool livepatch_fix2_dummy_check(struct dummy *d, u= nsigned long jiffies) * already have a SV_COUNTER shadow variable, then attach a * new one. */ - shadow_count =3D klp_shadow_get_or_alloc(d, SV_COUNTER, - sizeof(*shadow_count), GFP_NOWAIT, - NULL, NULL); + shadow_count =3D klp_shadow_get_or_alloc(d, &shadow_counter_type, + sizeof(*shadow_count), GFP_NOWAIT, NULL); if (shadow_count) *shadow_count +=3D 1; =20 @@ -72,9 +74,9 @@ static void livepatch_fix2_dummy_free(struct dummy *d) int *shadow_count; =20 /* Patch: copy the memory leak patch from the fix1 module. */ - shadow_leak =3D klp_shadow_get(d, SV_LEAK); + shadow_leak =3D klp_shadow_get(d, &shadow_leak_type); if (shadow_leak) - klp_shadow_free(d, SV_LEAK, livepatch_fix2_dummy_leak_dtor); + klp_shadow_free(d, &shadow_leak_type); else pr_info("%s: dummy @ %p leaked!\n", __func__, d); =20 @@ -82,16 +84,25 @@ static void livepatch_fix2_dummy_free(struct dummy *d) * Patch: fetch the SV_COUNTER shadow variable and display * the final count. Detach the shadow variable. */ - shadow_count =3D klp_shadow_get(d, SV_COUNTER); + shadow_count =3D klp_shadow_get(d, &shadow_counter_type); if (shadow_count) { pr_info("%s: dummy @ %p, check counter =3D %d\n", __func__, d, *shadow_count); - klp_shadow_free(d, SV_COUNTER, NULL); + klp_shadow_free(d, &shadow_counter_type); } =20 kfree(d); } =20 +static struct klp_shadow_type shadow_leak_type =3D { + .id =3D SV_LEAK, + .dtor =3D livepatch_fix2_dummy_leak_dtor, +}; + +static struct klp_shadow_type shadow_counter_type =3D { + .id =3D SV_COUNTER, +}; + static struct klp_func funcs[] =3D { { .old_name =3D "dummy_check", @@ -123,7 +134,7 @@ static int livepatch_shadow_fix2_init(void) static void livepatch_shadow_fix2_exit(void) { /* Cleanup any existing SV_COUNTER shadow variables */ - klp_shadow_free_all(SV_COUNTER, NULL); + klp_shadow_free_all(&shadow_leak_type); } =20 module_init(livepatch_shadow_fix2_init); diff --git a/tools/testing/selftests/livepatch/test-shadow-vars.sh b/tools/= testing/selftests/livepatch/test-shadow-vars.sh index e04cb354f56b..01ef65bc1f0c 100755 --- a/tools/testing/selftests/livepatch/test-shadow-vars.sh +++ b/tools/testing/selftests/livepatch/test-shadow-vars.sh @@ -67,7 +67,7 @@ $MOD_TEST: klp_shadow_get(obj=3DPTR9, id=3D0x1235) =3D PT= R11 $MOD_TEST: got expected PTR11 -> PTR10 result $MOD_TEST: klp_shadow_get(obj=3DPTR14, id=3D0x1235) =3D PTR16 $MOD_TEST: got expected PTR16 -> PTR15 result -$MOD_TEST: klp_shadow_free_all(id=3D0x1235, dtor=3DPTR0) +$MOD_TEST: klp_shadow_free_all(id=3D0x1235, dtor=3DPTR17) $MOD_TEST: klp_shadow_get(obj=3DPTR1, id=3D0x1235) =3D PTR0 $MOD_TEST: got expected NULL result $MOD_TEST: klp_shadow_get(obj=3DPTR9, id=3D0x1235) =3D PTR0 --=20 2.37.3 From nobody Wed Apr 8 14:05:22 2026 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9FA1AC433FE for ; Wed, 26 Oct 2022 19:42:09 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234369AbiJZTmH (ORCPT ); Wed, 26 Oct 2022 15:42:07 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39060 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234432AbiJZTlt (ORCPT ); Wed, 26 Oct 2022 15:41:49 -0400 Received: from smtp-out1.suse.de (smtp-out1.suse.de [IPv6:2001:67c:2178:6::1c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 6DE4EBC608; Wed, 26 Oct 2022 12:41:47 -0700 (PDT) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id DDCA722029; Wed, 26 Oct 2022 19:41:45 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1666813305; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=1Lk0jla7Dh3kqdUHZpu2RNb/LcgsWJikmOAgMB8CUfw=; b=TAfAc8VM1Vaf0+ZB5lC3dMQLSXTFUfue1NPEssJ+0NN/pyv8Di3AST3eqQ8Hc5h4anZVyY tU4HQLR1AHVZ1FAI+fz0XsWV1UIhc+C8IOUSyDInyo2vS7QdAYHNUT7VoM9ilfZMNbRawa B//bg2AVCQDEe8AdXK/5rFVNEzdUSdY= Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 1B9D813A77; Wed, 26 Oct 2022 19:41:42 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id IEzmNHaNWWNlFwAAMHmgww (envelope-from ); Wed, 26 Oct 2022 19:41:42 +0000 From: Marcos Paulo de Souza To: linux-kernel@vger.kernel.org, live-patching@vger.kernel.org Cc: jpoimboe@redhat.com, joe.lawrence@redhat.com, pmladek@suse.com, Marcos Paulo de Souza Subject: [PATCH v2 4/4] livepatch/shadow: Add garbage collection of shadow variables Date: Wed, 26 Oct 2022 16:41:22 -0300 Message-Id: <20221026194122.11761-5-mpdesouza@suse.com> X-Mailer: git-send-email 2.37.3 In-Reply-To: <20221026194122.11761-1-mpdesouza@suse.com> References: <20221026194122.11761-1-mpdesouza@suse.com> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Type: text/plain; charset="utf-8" The life of shadow variables is not completely trivial to maintain. They might be used by more livepatches and more livepatched objects. They should stay as long as there is any user. In practice, it requires to implement reference counting in callbacks of all users. They should register all the user and remove the shadow variables only when there is no user left. This patch hides the reference counting into the klp_shadow API. The counter is connected with the shadow variable @id. It requires an API to take and release the reference. The release function also calls the related dtor() when defined. An easy solution would be to add some get_ref()/put_ref() API. But it would need to get called from pre()/post_un() callbacks. It might be easy to forget a callback and make it wrong. A more safe approach is to associate the klp_shadow_type with klp_objects that use the shadow variables. The livepatch core code might then handle the reference counters on background. The shadow variable type might then be added into a new @shadow_types member of struct klp_object. They will get then automatically registered and unregistered when the object is being livepatched. The registration increments the reference count. Unregistration decreases the reference count. All shadow variables of the given type are freed when the reference count reaches zero. All klp_shadow_alloc/get/free functions also checks whether the requested type is registered. It will help to catch missing registration and might also help to catch eventual races. Signed-off-by: Petr Mladek Reviewed-by: Petr Mladek Signed-off-by: Marcos Paulo de Souza --- Changes from v1: * Reordered my SoB (Josh) * Changed code comments (Petr) include/linux/livepatch.h | 21 ++++ kernel/livepatch/core.c | 39 +++++++ kernel/livepatch/core.h | 1 + kernel/livepatch/shadow.c | 124 ++++++++++++++++++++++ kernel/livepatch/transition.c | 4 +- lib/livepatch/test_klp_shadow_vars.c | 18 +++- samples/livepatch/livepatch-shadow-fix1.c | 8 +- samples/livepatch/livepatch-shadow-fix2.c | 9 +- 8 files changed, 214 insertions(+), 10 deletions(-) diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h index 79e7bf3b35f6..fdd82fde86e6 100644 --- a/include/linux/livepatch.h +++ b/include/linux/livepatch.h @@ -100,11 +100,14 @@ struct klp_callbacks { bool post_unpatch_enabled; }; =20 +struct klp_shadow_type; + /** * struct klp_object - kernel object structure for live patching * @name: module name (or NULL for vmlinux) * @funcs: function entries for functions to be patched in the object * @callbacks: functions to be executed pre/post (un)patching + * @shadow_types: shadow variable types used by the livepatch for the klp_= object * @kobj: kobject for sysfs resources * @func_list: dynamic list of the function entries * @node: list node for klp_patch obj_list @@ -118,6 +121,7 @@ struct klp_object { const char *name; struct klp_func *funcs; struct klp_callbacks callbacks; + struct klp_shadow_type **shadow_types; =20 /* internal */ struct kobject kobj; @@ -222,13 +226,30 @@ typedef void (*klp_shadow_dtor_t)(void *obj, void *sh= adow_data); * @ctor: custom constructor to initialize the shadow data (optional) * @dtor: custom callback that can be used to unregister the variable * and/or free data that the shadow variable points to (optional) + * @registered: flag indicating if the variable was successfully registered + * + * All shadow variables used by the livepatch for the related klp_object + * must be listed here so that they are registered when the livepatch + * and the module is loaded. Otherwise, it will not be possible to + * allocate them. */ struct klp_shadow_type { unsigned long id; klp_shadow_ctor_t ctor; klp_shadow_dtor_t dtor; + + /* internal */ + bool registered; }; =20 +#define klp_for_each_shadow_type(obj, shadow_type, i) \ + for (shadow_type =3D obj->shadow_types ? obj->shadow_types[0] : NULL, i = =3D 1; \ + shadow_type; \ + shadow_type =3D obj->shadow_types[i++]) + +int klp_shadow_register(struct klp_shadow_type *shadow_type); +void klp_shadow_unregister(struct klp_shadow_type *shadow_type); + void *klp_shadow_get(void *obj, struct klp_shadow_type *shadow_type); void *klp_shadow_alloc(void *obj, struct klp_shadow_type *shadow_type, size_t size, gfp_t gfp_flags, void *ctor_data); diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c index 9ada0bc5247b..44c9e5ea0d2c 100644 --- a/kernel/livepatch/core.c +++ b/kernel/livepatch/core.c @@ -928,6 +928,30 @@ static int klp_init_patch(struct klp_patch *patch) return 0; } =20 +void klp_unregister_shadow_types(struct klp_object *obj) +{ + struct klp_shadow_type *shadow_type; + int i; + + klp_for_each_shadow_type(obj, shadow_type, i) { + klp_shadow_unregister(shadow_type); + } +} + +static int klp_register_shadow_types(struct klp_object *obj) +{ + struct klp_shadow_type *shadow_type; + int i, ret; + + klp_for_each_shadow_type(obj, shadow_type, i) { + ret =3D klp_shadow_register(shadow_type); + if (ret) + return ret; + } + + return 0; +} + static int __klp_disable_patch(struct klp_patch *patch) { struct klp_object *obj; @@ -988,6 +1012,13 @@ static int __klp_enable_patch(struct klp_patch *patch) if (!klp_is_object_loaded(obj)) continue; =20 + ret =3D klp_register_shadow_types(obj); + if (ret) { + pr_warn("failed to register shadow types for object '%s'\n", + klp_is_module(obj) ? obj->name : "vmlinux"); + goto err; + } + ret =3D klp_pre_patch_callback(obj); if (ret) { pr_warn("pre-patch callback failed for object '%s'\n", @@ -1172,6 +1203,7 @@ static void klp_cleanup_module_patches_limited(struct= module *mod, klp_unpatch_object(obj); =20 klp_post_unpatch_callback(obj); + klp_unregister_shadow_types(obj); =20 klp_free_object_loaded(obj); break; @@ -1218,6 +1250,13 @@ int klp_module_coming(struct module *mod) pr_notice("applying patch '%s' to loading module '%s'\n", patch->mod->name, obj->mod->name); =20 + ret =3D klp_register_shadow_types(obj); + if (ret) { + pr_warn("failed to register shadow types for object '%s'\n", + obj->name); + goto err; + } + ret =3D klp_pre_patch_callback(obj); if (ret) { pr_warn("pre-patch callback failed for object '%s'\n", diff --git a/kernel/livepatch/core.h b/kernel/livepatch/core.h index 38209c7361b6..0b68f2407a82 100644 --- a/kernel/livepatch/core.h +++ b/kernel/livepatch/core.h @@ -13,6 +13,7 @@ extern struct list_head klp_patches; #define klp_for_each_patch(patch) \ list_for_each_entry(patch, &klp_patches, list) =20 +void klp_unregister_shadow_types(struct klp_object *obj); void klp_free_patch_async(struct klp_patch *patch); void klp_free_replaced_patches_async(struct klp_patch *new_patch); void klp_unpatch_replaced_patches(struct klp_patch *new_patch); diff --git a/kernel/livepatch/shadow.c b/kernel/livepatch/shadow.c index 64e83853891d..9437dc1be7b2 100644 --- a/kernel/livepatch/shadow.c +++ b/kernel/livepatch/shadow.c @@ -34,6 +34,7 @@ #include #include #include +#include "core.h" =20 static DEFINE_HASHTABLE(klp_shadow_hash, 12); =20 @@ -59,6 +60,22 @@ struct klp_shadow { char data[]; }; =20 +/** + * struct klp_shadow_type_reg - information about a registered shadow + * variable type + * @id: shadow variable type indentifier + * @count: reference counter + * @list: list node for list of registered shadow variable types + */ +struct klp_shadow_type_reg { + unsigned long id; + int ref_cnt; + struct list_head list; +}; + +/* List of registered shadow variable types */ +static LIST_HEAD(klp_shadow_types); + /** * klp_shadow_match() - verify a shadow variable matches given * @shadow: shadow variable to match @@ -84,6 +101,13 @@ void *klp_shadow_get(void *obj, struct klp_shadow_type = *shadow_type) { struct klp_shadow *shadow; =20 + /* Just the best effort. Can't take @klp_shadow_lock here. */ + if (!shadow_type->registered) { + pr_err("Trying to get shadow variable of non-registered type: %lu\n", + shadow_type->id); + return NULL; + } + rcu_read_lock(); =20 hash_for_each_possible_rcu(klp_shadow_hash, shadow, node, @@ -310,3 +334,103 @@ void klp_shadow_free_all(struct klp_shadow_type *shad= ow_type) spin_unlock_irqrestore(&klp_shadow_lock, flags); } EXPORT_SYMBOL_GPL(klp_shadow_free_all); + +static struct klp_shadow_type_reg * +klp_shadow_type_get_reg(struct klp_shadow_type *shadow_type) +{ + struct klp_shadow_type_reg *shadow_type_reg; + lockdep_assert_held(&klp_shadow_lock); + + list_for_each_entry(shadow_type_reg, &klp_shadow_types, list) { + if (shadow_type_reg->id =3D=3D shadow_type->id) + return shadow_type_reg; + } + + return NULL; +} + +/** + * klp_shadow_register() - register the given shadow variable type + * @shadow_type: shadow type to be registered + * + * Tell the system that the given shadow type is going to used by the call= er + * (livepatch module). It allows to check and maintain lifetime of shadow + * variables. + * + * Return: 0 on suceess, -ENOMEM when there is not enough memory. + */ +int klp_shadow_register(struct klp_shadow_type *shadow_type) +{ + struct klp_shadow_type_reg *shadow_type_reg; + struct klp_shadow_type_reg *new_shadow_type_reg; + + new_shadow_type_reg =3D + kzalloc(sizeof(struct klp_shadow_type_reg), GFP_KERNEL); + if (!new_shadow_type_reg) + return -ENOMEM; + + spin_lock_irq(&klp_shadow_lock); + + if (shadow_type->registered) { + pr_err("Trying to register shadow variable type that is already register= ed: %lu", + shadow_type->id); + kfree(new_shadow_type_reg); + goto out; + } + + shadow_type_reg =3D klp_shadow_type_get_reg(shadow_type); + if (!shadow_type_reg) { + shadow_type_reg =3D new_shadow_type_reg; + shadow_type_reg->id =3D shadow_type->id; + list_add(&shadow_type_reg->list, &klp_shadow_types); + } else { + kfree(new_shadow_type_reg); + } + + shadow_type_reg->ref_cnt++; + shadow_type->registered =3D true; +out: + spin_unlock_irq(&klp_shadow_lock); + + return 0; +} +EXPORT_SYMBOL_GPL(klp_shadow_register); + +/** + * klp_shadow_unregister() - unregister the give shadow variable type + * @shadow_type: shadow type to be unregistered + * + * Tell the system that a given shadow variable ID is not longer be used by + * the caller (livepatch module). All existing shadow variables are freed + * when it was the last registered user. + */ +void klp_shadow_unregister(struct klp_shadow_type *shadow_type) +{ + struct klp_shadow_type_reg *shadow_type_reg; + + spin_lock_irq(&klp_shadow_lock); + + if (!shadow_type->registered) { + pr_err("Trying to unregister shadow variable type that is not registered= : %lu", + shadow_type->id); + goto out; + } + + shadow_type_reg =3D klp_shadow_type_get_reg(shadow_type); + if (!shadow_type_reg) { + pr_err("Can't find shadow variable type registration: %lu", shadow_type-= >id); + goto out; + } + + shadow_type->registered =3D false; + shadow_type_reg->ref_cnt--; + + if (!shadow_type_reg->ref_cnt) { + __klp_shadow_free_all(shadow_type); + list_del(&shadow_type_reg->list); + kfree(shadow_type_reg); + } +out: + spin_unlock_irq(&klp_shadow_lock); +} +EXPORT_SYMBOL_GPL(klp_shadow_unregister); diff --git a/kernel/livepatch/transition.c b/kernel/livepatch/transition.c index 30187b1d8275..9c57941974a7 100644 --- a/kernel/livepatch/transition.c +++ b/kernel/livepatch/transition.c @@ -123,8 +123,10 @@ static void klp_complete_transition(void) continue; if (klp_target_state =3D=3D KLP_PATCHED) klp_post_patch_callback(obj); - else if (klp_target_state =3D=3D KLP_UNPATCHED) + else if (klp_target_state =3D=3D KLP_UNPATCHED) { klp_post_unpatch_callback(obj); + klp_unregister_shadow_types(obj); + } } =20 pr_notice("'%s': %s complete\n", klp_transition_patch->mod->name, diff --git a/lib/livepatch/test_klp_shadow_vars.c b/lib/livepatch/test_klp_= shadow_vars.c index ee47c1fae8e2..6de1f6d11bcf 100644 --- a/lib/livepatch/test_klp_shadow_vars.c +++ b/lib/livepatch/test_klp_shadow_vars.c @@ -188,6 +188,17 @@ static int test_klp_shadow_vars_init(void) int ret; int i; =20 + /* Registered manually since we don't have a klp_object instance. */ + ret =3D klp_shadow_register(&shadow_type_1); + if (ret) + return ret; + + ret =3D klp_shadow_register(&shadow_type_2); + if (ret) { + klp_shadow_unregister(&shadow_type_1); + return ret; + } + ptr_id(NULL); =20 /* @@ -296,12 +307,9 @@ static int test_klp_shadow_vars_init(void) pr_info(" got expected NULL result\n"); } =20 - free_ptr_list(); - - return 0; out: - shadow_free_all(&shadow_type_1); /* 'char' pairs */ - shadow_free_all(&shadow_type_2); /* 'int' pairs */ + klp_shadow_unregister(&shadow_type_1); /* 'char' pairs */ + klp_shadow_unregister(&shadow_type_2); /* 'int' pairs */ free_ptr_list(); =20 return ret; diff --git a/samples/livepatch/livepatch-shadow-fix1.c b/samples/livepatch/= livepatch-shadow-fix1.c index 0cc7d1e4b4bc..6718df9ec14b 100644 --- a/samples/livepatch/livepatch-shadow-fix1.c +++ b/samples/livepatch/livepatch-shadow-fix1.c @@ -141,6 +141,11 @@ static struct klp_shadow_type shadow_leak_type =3D { .dtor =3D livepatch_fix1_dummy_leak_dtor, }; =20 +struct klp_shadow_type *shadow_types[] =3D { + &shadow_leak_type, + NULL +}; + static struct klp_func funcs[] =3D { { .old_name =3D "dummy_alloc", @@ -156,6 +161,7 @@ static struct klp_object objs[] =3D { { .name =3D "livepatch_shadow_mod", .funcs =3D funcs, + .shadow_types =3D shadow_types, }, { } }; =20 @@ -171,8 +177,6 @@ static int livepatch_shadow_fix1_init(void) =20 static void livepatch_shadow_fix1_exit(void) { - /* Cleanup any existing SV_LEAK shadow variables */ - klp_shadow_free_all(&shadow_leak_type); } =20 module_init(livepatch_shadow_fix1_init); diff --git a/samples/livepatch/livepatch-shadow-fix2.c b/samples/livepatch/= livepatch-shadow-fix2.c index 840100555152..290c7e3f96b0 100644 --- a/samples/livepatch/livepatch-shadow-fix2.c +++ b/samples/livepatch/livepatch-shadow-fix2.c @@ -103,6 +103,12 @@ static struct klp_shadow_type shadow_counter_type =3D { .id =3D SV_COUNTER, }; =20 +struct klp_shadow_type *shadow_types[] =3D { + &shadow_leak_type, + &shadow_counter_type, + NULL +}; + static struct klp_func funcs[] =3D { { .old_name =3D "dummy_check", @@ -118,6 +124,7 @@ static struct klp_object objs[] =3D { { .name =3D "livepatch_shadow_mod", .funcs =3D funcs, + .shadow_types =3D shadow_types, }, { } }; =20 @@ -133,8 +140,6 @@ static int livepatch_shadow_fix2_init(void) =20 static void livepatch_shadow_fix2_exit(void) { - /* Cleanup any existing SV_COUNTER shadow variables */ - klp_shadow_free_all(&shadow_leak_type); } =20 module_init(livepatch_shadow_fix2_init); --=20 2.37.3