From patchwork Wed Oct 26 19:41:19 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marcos Paulo de Souza X-Patchwork-Id: 11413 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:6687:0:0:0:0:0 with SMTP id l7csp453543wru; Wed, 26 Oct 2022 12:45:08 -0700 (PDT) X-Google-Smtp-Source: AMsMyM7xgdmzyuejh09HtAFYHXi2YAJ+I5Ypq6IoM9IJaaiKLfpSiIm8K7iprIr/1znWcKbhawfb X-Received: by 2002:a17:902:cf08:b0:17d:46b6:25f9 with SMTP id i8-20020a170902cf0800b0017d46b625f9mr45795378plg.67.1666813497313; Wed, 26 Oct 2022 12:44:57 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1666813497; cv=none; d=google.com; s=arc-20160816; b=ss/d6uOH/AXOwg8YN3XNOyeS5Ei8EcZBXV2AX4r4KWa0UMTnYYZwa+uRsWNGTbDODC whV1jB34KbaxRyOnvU7zL8pMpYkKEIk+eFl/G0wcQQ7F20eZPfmMVS72wRrJ4NR1nfxb UULv+XF8uHmrZ9y5xbxqM++mWr7aKl5vF1oHT04xPFHhyo63zrIdFzHU/bWEupZadx9Q Te4Kd7K60pEkcGeN8Xy68zLQULM0Pnvn52433MBFYiuYoWOb8XdlgdrDRE175gsH//WT 7kyTOANSQTdrGLMttBKS5Sq5tsBmSNbqKNmBWzHGegf92xCEwGwmhC7iZYCwFDdoATO6 +S0g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=R2ZOrYe1VpBXYn04M+t8OPuDPgxBa02cJDnMZ+dKsEs=; b=FDAPxk6PEclPga/lv9ellb1hc3+WIxN6lMTfkG8pXQb94TP6inpeS35TLuImlGON0I +ONjTlXlESH2ZfI8hfqOKoHTl5Sm2dA0vm/VOnI6uSDCRntHKawOeqvG4UhrFjB6dbl7 8xmzr9+fmwuHbCsuZusFmGWI3jeUPyDNZywBuVIAt010fKtYajaSIBkvTzJUNNQvKzEr TE0RxTufSGQLCf+SRhBTJTw1QZjHl0oovol87NiOEG4efr3AuDpbhEq6EQpeNzLhGy6z pWqiLYL7JZiYzLxMs+IPSmuwL2pxUa18k9ynuMHMiF13cX6j7Sk2iuaC/sX4mFJ0LMmx wtGg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@suse.com header.s=susede1 header.b="M+kae/Jv"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=suse.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id l3-20020a056a00140300b0056c12518664si8528858pfu.128.2022.10.26.12.44.44; Wed, 26 Oct 2022 12:44:57 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@suse.com header.s=susede1 header.b="M+kae/Jv"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=suse.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234845AbiJZTlw (ORCPT + 99 others); Wed, 26 Oct 2022 15:41:52 -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 X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1747780629754132838?= X-GMAIL-MSGID: =?utf-8?q?1747780629754132838?= 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); -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; - /* Check if the shadow variable already exists */ - shadow_data = 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 = kzalloc(size + sizeof(*new_shadow), gfp_flags); - if (!new_shadow) - return NULL; + lockdep_assert_held(&klp_shadow_lock); - /* Look for again under the lock */ - spin_lock_irqsave(&klp_shadow_lock, flags); shadow_data = 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 = true; + return shadow_data; } + *exist = false; new_shadow->obj = obj; new_shadow->id = id; @@ -145,8 +126,6 @@ static void *__klp_shadow_get_or_alloc(void *obj, unsigned long id, err = 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, unsigned 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); 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 = 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 = 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 = __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); -exists: - if (warn_on_exist) { + if (exist && warn_on_exist) { WARN(1, "Duplicate shadow variable <%p, %lx>\n", obj, id); return NULL; } From patchwork Wed Oct 26 19:41:20 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marcos Paulo de Souza X-Patchwork-Id: 11408 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:6687:0:0:0:0:0 with SMTP id l7csp453167wru; Wed, 26 Oct 2022 12:44:02 -0700 (PDT) X-Google-Smtp-Source: AMsMyM6IBB5Okv4fGsNVJgrfA4uxTYW5ZRTziWgYYqEJXfoSbRxYKnXaTF5VTC/O5GW8+X00gTO8 X-Received: by 2002:a17:902:a713:b0:183:e2a9:6409 with SMTP id w19-20020a170902a71300b00183e2a96409mr44766930plq.149.1666813442411; Wed, 26 Oct 2022 12:44:02 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1666813442; cv=none; d=google.com; s=arc-20160816; b=ldMOnh5AbaQj0ANQD1qaXPrcF5o8rpYEZM1ODC9Od0ItLMpVFEnfKoueBMcwmIwiyw HYtj4C18/JWW/88ItarVxKFSJIwRHCiOZCSFJ7FAKaaoDTkpiRFgMKG+pJan+rfW0a+a 6FYAwBBSUH3Zwj2EdybvAJK2hKkx7Oyr7VEQ3dmoKMltP/WFGm5i3Uvh02wTL/jPy/PM yCDSR2CUb0w2NCkiqS4Z3aZ1K9pUbQybxbKW1p/54VCoCir27DGTSLApTFSb+0KJCGvd 3m+VH8Izkvy53qH5z2JKnA1icgSPlON8ty0Ke78YWhJwxFHQ519GpVqtN8ToV+kxGGbQ TlAw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=XaNif8Cw4uustU+ovZVF6G4VJf04B2ti3O7xS7XvquQ=; b=CkCbpeFzGO2xLdI8MJsqZa6TikOKUDsX0tmr9LSTzHMhnMKG9iLr9GnSo8J+ZFvm9h c++hZfOWV2934GpNyIfytLZtBYhfqx9nIe/zc8SEMXrwIBXvQuE4ppqT+gJR/IVW71Eq Bm2GEZrrbco63eF5r9ePuVmFNXt5T7fOLq5k6vEmJf5e8EZyCyQXODVRj7PhtKhqUVLo VNVsk8tE5iMZgxHQx7ovf4xcFK8lWt5HJUr5VrHOCdGHTiRy2hLtZiYn0M5wvpCkqMAI 0kxqNa9WH8SXO4riCZDhal3hVb2Cap1c7OpArYEhk0ahI366kuaUWsBbZ5X3tmFFIX0b ygYA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@suse.com header.s=susede1 header.b=SvwwGe9C; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=suse.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id b20-20020a656694000000b0044af51e7d4bsi8450645pgw.75.2022.10.26.12.43.48; Wed, 26 Oct 2022 12:44:02 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@suse.com header.s=susede1 header.b=SvwwGe9C; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=suse.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234899AbiJZTl5 (ORCPT + 99 others); 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 X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1747780571816085241?= X-GMAIL-MSGID: =?utf-8?q?1747780571816085241?= 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); +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; 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); From patchwork Wed Oct 26 19:41:21 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marcos Paulo de Souza X-Patchwork-Id: 11409 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:6687:0:0:0:0:0 with SMTP id l7csp453277wru; Wed, 26 Oct 2022 12:44:16 -0700 (PDT) X-Google-Smtp-Source: AMsMyM41eK2WGJJV98AY+fUoa4ltfkBYWLmdG53j7cOW3cTZJLMLgiLrGPymNUw9+AKOtmqmjxs4 X-Received: by 2002:a17:907:2672:b0:734:a952:439a with SMTP id ci18-20020a170907267200b00734a952439amr36562171ejc.539.1666813456736; Wed, 26 Oct 2022 12:44:16 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1666813456; cv=none; d=google.com; s=arc-20160816; b=DfkLd4g9dWBXrZbkIj2wawGUeQW3fcRyO1ltTqa2ZHEq/T63Epkh0V01ys5f6T65Xt 7QsJQGebELGNFJFRA5pt1SPQj7rcd4ZbhcoYqn/vu3ISa8gRc5+yFlftljj6wchxeycC B3EOC7/vIVcbco9KHEBQsv4icos2DsgcNFkE9GmYluv2jwx+poiSk98u4yqDkXD2Duad 1QcBb57qgSn8GRbY4v6o5Cwl4R6HgG04Ccryi0YzsC19FlIGgD+rDLTr7B9uuuBiMlwp Ut28otdbbrAzOU2atTF9I5YOTSkBu0NyhwVttMJ0feeuM5KIQAH00vK2/t6moMC0EIGl wAXA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=Gei3z6v/UM+glPIhIg6yXmE+bgYZJKhOzcfEW7UYnXE=; b=isuqomfqoAYQAaYJ1WCsoon01ZPK9RKk9g2XX92eYkK5BL0288b/s2nl8wf/nGHWMu Zc+xe87E2BaMFrUEar5B/AvTtQfCrdt62m6GzM3keneF23i9UiO/bapgQ9at7PndBODd X4n3FnQHHzM2GFvz1murmiKq8UhTidp461gaXty773oZmVbtVBpjsEfdENBCjJ7Pgkex 2TTCMr2OBB2/PoUvnaU8fTWCT/SU7yOPgOUKgC0KWH7IW4JJBvlu/n6y6teublnmgABR HkoMcYRoANG+upc1JHzoUqhhdsmbfTtojlmkeinMmJfg28Zuck1kekgjSWMdqYVfko/F Dy/A== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@suse.com header.s=susede1 header.b=N2iFmJwA; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=suse.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id h4-20020a170906854400b007330c08fe49si5666562ejy.206.2022.10.26.12.43.50; Wed, 26 Oct 2022 12:44:16 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@suse.com header.s=susede1 header.b=N2iFmJwA; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=suse.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233536AbiJZTmB (ORCPT + 99 others); Wed, 26 Oct 2022 15:42:01 -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 X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1747780587332228302?= X-GMAIL-MSGID: =?utf-8?q?1747780587332228302?= 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 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); -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_type, + 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); 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 == obj && shadow->id == id; + return shadow->obj == obj && shadow->id == shadow_type->id; } /** * 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; @@ -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) { - 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); -/* 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; lockdep_assert_held(&klp_shadow_lock); - shadow_data = klp_shadow_get(obj, id); + shadow_data = klp_shadow_get(obj, shadow_type); if (unlikely(shadow_data)) { *exist = true; return shadow_data; @@ -119,15 +118,15 @@ static void *__klp_shadow_get_or_add_locked(void *obj, unsigned long id, *exist = false; new_shadow->obj = obj; - new_shadow->id = id; + new_shadow->id = shadow_type->id; - if (ctor) { + if (shadow_type->ctor) { int err; - err = ctor(obj, new_shadow->data, ctor_data); + err = 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; } -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, unsigned long id, unsigned long flags; /* Check if the shadow variable already exists */ - shadow_data = klp_shadow_get(obj, id); + shadow_data = klp_shadow_get(obj, shadow_type); if (shadow_data) return shadow_data; @@ -159,22 +157,25 @@ static void *__klp_shadow_get_or_alloc(void *obj, unsigned 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 = kzalloc(size + sizeof(*new_shadow), gfp_flags); + new_shadow = kzalloc(size + sizeof(struct klp_shadow), gfp_flags); if (!new_shadow) return NULL; /* Look for again under the lock */ spin_lock_irqsave(&klp_shadow_lock, flags); - shadow_data = __klp_shadow_get_or_add_locked(obj, id, new_shadow, - ctor, ctor_data, &exist); + shadow_data = __klp_shadow_get_or_add_locked(obj, shadow_type, + new_shadow, ctor_data, &exist); spin_unlock_irqrestore(&klp_shadow_lock, flags); - /* 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); 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; } @@ -184,10 +185,9 @@ static void *__klp_shadow_get_or_alloc(void *obj, unsigned 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, unsigned 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); /** * klp_shadow_get_or_alloc() - get existing or allocate 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) * * 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_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, false); + return __klp_shadow_get_or_alloc(obj, shadow_type, size, + gfp_flags, ctor_data, + false); } EXPORT_SYMBOL_GPL(klp_shadow_get_or_alloc); 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); } /** * 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_shadow_dtor_t dtor) hash_for_each_possible(klp_shadow_hash, shadow, node, (unsigned long)obj) { - 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_shadow_dtor_t dtor) } EXPORT_SYMBOL_GPL(klp_shadow_free); -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, klp_shadow_dtor_t dtor) /* 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); } } /** * 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; 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; - sv = klp_shadow_get(obj, id); + sv = klp_shadow_get(obj, shadow_type); pr_info("klp_%s(obj=PTR%d, id=0x%lx) = PTR%d\n", - __func__, ptr_id(obj), id, ptr_id(sv)); + __func__, ptr_id(obj), shadow_type->id, ptr_id(sv)); return sv; } -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 = ctor_data; int **sv; - sv = klp_shadow_alloc(obj, id, size, gfp_flags, ctor, var); + sv = klp_shadow_alloc(obj, shadow_type, size, gfp_flags, var); pr_info("klp_%s(obj=PTR%d, id=0x%lx, size=%zx, gfp_flags=%pGg), ctor=PTR%d, ctor_data=PTR%d = 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)); return sv; } -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 = ctor_data; int **sv; - sv = klp_shadow_get_or_alloc(obj, id, size, gfp_flags, ctor, var); + sv = klp_shadow_get_or_alloc(obj, shadow_type, size, gfp_flags, var); pr_info("klp_%s(obj=PTR%d, id=0x%lx, size=%zx, gfp_flags=%pGg), ctor=PTR%d, ctor_data=PTR%d = 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)); return sv; } -static void shadow_free(void *obj, unsigned long id, klp_shadow_dtor_t dtor) +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=PTR%d, id=0x%lx, dtor=PTR%d)\n", - __func__, ptr_id(obj), id, ptr_id(dtor)); + __func__, ptr_id(obj), shadow_type->id, ptr_id(shadow_type->dtor)); } -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 = true; +static void shadow_free_all(struct klp_shadow_type *shadow_type) { - klp_shadow_free_all(id, dtor); - pr_info("klp_%s(id=0x%lx, dtor=PTR%d)\n", __func__, id, ptr_id(dtor)); + verbose_dtor = false; + klp_shadow_free_all(shadow_type); + verbose_dtor = true; + pr_info("klp_%s(id=0x%lx, dtor=PTR%d)\n", __func__, shadow_type->id, ptr_id(shadow_type->dtor)); } @@ -128,17 +134,14 @@ static int shadow_ctor(void *obj, void *shadow_data, void *ctor_data) return 0; } -/* - * 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 = shadow_data; - pr_info("%s(obj=PTR%d, shadow_data=PTR%d)\n", - __func__, ptr_id(obj), ptr_id(sv)); + if (verbose_dtor) { + pr_info("%s(obj=PTR%d, shadow_data=PTR%d)\n", + __func__, ptr_id(obj), ptr_id(sv)); + } } /* 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 +struct klp_shadow_type shadow_type_1 = { + .id = SV_ID1, + .ctor = shadow_ctor, + .dtor = shadow_dtor, +}; + +struct klp_shadow_type shadow_type_2 = { + .id = SV_ID2, + .ctor = shadow_ctor, + .dtor = 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 represent @@ -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 = shadow_get(&objs[0], SV_ID1); + sv = shadow_get(&objs[0], &shadow_type_1); if (!sv) pr_info(" got expected NULL result\n"); @@ -189,13 +204,13 @@ static int test_klp_shadow_vars_init(void) ptr_id(pnfields1[i]); if (i % 2) { - sv1[i] = shadow_alloc(&objs[i], SV_ID1, + sv1[i] = shadow_alloc(&objs[i], &shadow_type_1, sizeof(pnfields1[i]), GFP_KERNEL, - shadow_ctor, &pnfields1[i]); + &pnfields1[i]); } else { - sv1[i] = shadow_get_or_alloc(&objs[i], SV_ID1, + sv1[i] = shadow_get_or_alloc(&objs[i], &shadow_type_1, sizeof(pnfields1[i]), GFP_KERNEL, - shadow_ctor, &pnfields1[i]); + &pnfields1[i]); } if (!sv1[i]) { ret = -ENOMEM; @@ -204,8 +219,9 @@ static int test_klp_shadow_vars_init(void) pnfields2[i] = &nfields2[i]; ptr_id(pnfields2[i]); - sv2[i] = shadow_alloc(&objs[i], SV_ID2, sizeof(pnfields2[i]), - GFP_KERNEL, shadow_ctor, &pnfields2[i]); + sv2[i] = shadow_alloc(&objs[i], &shadow_type_2, + sizeof(pnfields2[i]), + GFP_KERNEL, &pnfields2[i]); if (!sv2[i]) { ret = -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 = 0; i < NUM_OBJS; i++) { /* check the "char" svar for all objects */ - sv = shadow_get(&objs[i], SV_ID1); + sv = shadow_get(&objs[i], &shadow_type_1); if (!sv) { ret = -EINVAL; goto out; @@ -225,7 +241,7 @@ static int test_klp_shadow_vars_init(void) ptr_id(sv1[i]), ptr_id(*sv1[i])); /* check the "int" svar for all objects */ - sv = shadow_get(&objs[i], SV_ID2); + sv = shadow_get(&objs[i], &shadow_type_2); if (!sv) { ret = -EINVAL; goto out; @@ -240,8 +256,9 @@ static int test_klp_shadow_vars_init(void) pndup[i] = &nfields1[i]; ptr_id(pndup[i]); - sv = shadow_get_or_alloc(&objs[i], SV_ID1, sizeof(pndup[i]), - GFP_KERNEL, shadow_ctor, &pndup[i]); + sv = shadow_get_or_alloc(&objs[i], &shadow_type_1, + sizeof(pndup[i]), + GFP_KERNEL, &pndup[i]); if (!sv) { ret = -EINVAL; goto out; @@ -253,15 +270,15 @@ static int test_klp_shadow_vars_init(void) /* pass 4: free pairs of svars, verify removal */ for (i = 0; i < NUM_OBJS; i++) { - shadow_free(&objs[i], SV_ID1, shadow_dtor); /* 'char' pairs */ - sv = shadow_get(&objs[i], SV_ID1); + shadow_free(&objs[i], &shadow_type_1); /* 'char' pairs */ + sv = shadow_get(&objs[i], &shadow_type_1); if (!sv) pr_info(" got expected NULL result\n"); } /* pass 5: check we still find svar pairs */ for (i = 0; i < NUM_OBJS; i++) { - sv = shadow_get(&objs[i], SV_ID2); /* 'int' pairs */ + sv = shadow_get(&objs[i], &shadow_type_2); /* 'int' pairs */ if (!sv) { ret = -EINVAL; goto out; @@ -272,9 +289,9 @@ static int test_klp_shadow_vars_init(void) } /* 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 = 0; i < NUM_OBJS; i++) { - sv = shadow_get(&objs[i], SV_ID2); + sv = 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) 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(); 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 +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; - shadow_leak = klp_shadow_alloc(d, SV_LEAK, sizeof(leak), GFP_KERNEL, - shadow_leak_ctor, &leak); + shadow_leak = 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 = klp_shadow_get(d, SV_LEAK); + shadow_leak = 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); kfree(d); } +static struct klp_shadow_type shadow_leak_type = { + .id = SV_LEAK, + .ctor = shadow_leak_ctor, + .dtor = livepatch_fix1_dummy_leak_dtor, +}; + static struct klp_func funcs[] = { { .old_name = "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); } 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 +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, unsigned long jiffies) * already have a SV_COUNTER shadow variable, then attach a * new one. */ - shadow_count = klp_shadow_get_or_alloc(d, SV_COUNTER, - sizeof(*shadow_count), GFP_NOWAIT, - NULL, NULL); + shadow_count = klp_shadow_get_or_alloc(d, &shadow_counter_type, + sizeof(*shadow_count), GFP_NOWAIT, NULL); if (shadow_count) *shadow_count += 1; @@ -72,9 +74,9 @@ static void livepatch_fix2_dummy_free(struct dummy *d) int *shadow_count; /* Patch: copy the memory leak patch from the fix1 module. */ - shadow_leak = klp_shadow_get(d, SV_LEAK); + shadow_leak = 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); @@ -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 = klp_shadow_get(d, SV_COUNTER); + shadow_count = klp_shadow_get(d, &shadow_counter_type); if (shadow_count) { pr_info("%s: dummy @ %p, check counter = %d\n", __func__, d, *shadow_count); - klp_shadow_free(d, SV_COUNTER, NULL); + klp_shadow_free(d, &shadow_counter_type); } kfree(d); } +static struct klp_shadow_type shadow_leak_type = { + .id = SV_LEAK, + .dtor = livepatch_fix2_dummy_leak_dtor, +}; + +static struct klp_shadow_type shadow_counter_type = { + .id = SV_COUNTER, +}; + static struct klp_func funcs[] = { { .old_name = "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); } 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=PTR9, id=0x1235) = PTR11 $MOD_TEST: got expected PTR11 -> PTR10 result $MOD_TEST: klp_shadow_get(obj=PTR14, id=0x1235) = PTR16 $MOD_TEST: got expected PTR16 -> PTR15 result -$MOD_TEST: klp_shadow_free_all(id=0x1235, dtor=PTR0) +$MOD_TEST: klp_shadow_free_all(id=0x1235, dtor=PTR17) $MOD_TEST: klp_shadow_get(obj=PTR1, id=0x1235) = PTR0 $MOD_TEST: got expected NULL result $MOD_TEST: klp_shadow_get(obj=PTR9, id=0x1235) = PTR0 From patchwork Wed Oct 26 19:41:22 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marcos Paulo de Souza X-Patchwork-Id: 11410 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:6687:0:0:0:0:0 with SMTP id l7csp453358wru; Wed, 26 Oct 2022 12:44:33 -0700 (PDT) X-Google-Smtp-Source: AMsMyM6DSaH9l/P7y3I9NVcEm8FTRUNyiuYIS2TWLzU7daND2isciZrAXDnxPdIP/RVkKNruVW5s X-Received: by 2002:a17:906:9bc4:b0:7aa:9184:d95d with SMTP id de4-20020a1709069bc400b007aa9184d95dmr13075394ejc.349.1666813473235; Wed, 26 Oct 2022 12:44:33 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1666813473; cv=none; d=google.com; s=arc-20160816; b=MTZHkT+UoI6VwgdAMDEBlLiNvLoZcBivc0Xbpo8aBpCKQduVN4I8g+r8H0GCGFwIcl 4kCbaGE2oIE8vFFhjGpAQaZDz3nuCpNvSA52U5rrSseYsTSEtqITitwUnovm8pk7Bjt3 BTaE4t1ZLNC7DsJBJWRuHGF16b4SHbTC2ZOLIHLlUgzyxXusNuHeURL0BCaRup9iwUJu 1yaMmUMNTM9Q7mxbhrSH9Tapqtd3RZgi/0Rz+dH3x8Xw6md1Ng3Nbk4I2cqNa5WBKIrg 0zkJbHi6IdJNkG5XFkKCY0XksBbcegSFYsc+jBKxPpC1i+JGKpMT9jl1LEBaCkXz2cIl LKfw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=1Lk0jla7Dh3kqdUHZpu2RNb/LcgsWJikmOAgMB8CUfw=; b=UNNarmYoMp4ptSi7Ze2ul3F/PLsQ/c0R0XsH28xk7rqAk3iZhTMiEyXFa0uJb+qasC TVzevnUw1KpXdNym89cUVQl59Dtw2Tsagf7/iv52iq0ncTdPi6V6ruvE1I1BHI2EIAVF I4rJqoIaOJ5ZgsPNCEw0vrxDvp7yd/fUr/2ifr3bmdocl91Qo50ftpVasW0nb+Zy6Wpq CaoE/6LV34hvJOfBlZcfZLMuHzAoAeDSAfn5ZI0xtd5P7okwj50CiuePO77Jxn4V71Gn de7DVgWzWFn01TEDyvyVth71jGbCIZxMq9xwD09Mo913Wn2NpggjoKmPhXc/tW3o4tvw 0FLg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@suse.com header.s=susede1 header.b=TAfAc8VM; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=suse.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id hz1-20020a1709072ce100b007acbaef6d9csi5182062ejc.137.2022.10.26.12.43.52; Wed, 26 Oct 2022 12:44:33 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@suse.com header.s=susede1 header.b=TAfAc8VM; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=suse.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233165AbiJZTmG (ORCPT + 99 others); Wed, 26 Oct 2022 15:42:06 -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 X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1747780604281836150?= X-GMAIL-MSGID: =?utf-8?q?1747780604281836150?= 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; }; +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; /* internal */ struct kobject kobj; @@ -222,13 +226,30 @@ typedef void (*klp_shadow_dtor_t)(void *obj, void *shadow_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; }; +#define klp_for_each_shadow_type(obj, shadow_type, i) \ + for (shadow_type = obj->shadow_types ? obj->shadow_types[0] : NULL, i = 1; \ + shadow_type; \ + shadow_type = 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; } +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 = 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; + ret = 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 = 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); klp_post_unpatch_callback(obj); + klp_unregister_shadow_types(obj); 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); + ret = klp_register_shadow_types(obj); + if (ret) { + pr_warn("failed to register shadow types for object '%s'\n", + obj->name); + goto err; + } + ret = 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) +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" static DEFINE_HASHTABLE(klp_shadow_hash, 12); @@ -59,6 +60,22 @@ struct klp_shadow { char data[]; }; +/** + * 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; + /* 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(); hash_for_each_possible_rcu(klp_shadow_hash, shadow, node, @@ -310,3 +334,103 @@ void klp_shadow_free_all(struct klp_shadow_type *shadow_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 == 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 caller + * (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 = + 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 registered: %lu", + shadow_type->id); + kfree(new_shadow_type_reg); + goto out; + } + + shadow_type_reg = klp_shadow_type_get_reg(shadow_type); + if (!shadow_type_reg) { + shadow_type_reg = new_shadow_type_reg; + shadow_type_reg->id = 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 = 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 = 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 = 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 == KLP_PATCHED) klp_post_patch_callback(obj); - else if (klp_target_state == KLP_UNPATCHED) + else if (klp_target_state == KLP_UNPATCHED) { klp_post_unpatch_callback(obj); + klp_unregister_shadow_types(obj); + } } 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; + /* Registered manually since we don't have a klp_object instance. */ + ret = klp_shadow_register(&shadow_type_1); + if (ret) + return ret; + + ret = klp_shadow_register(&shadow_type_2); + if (ret) { + klp_shadow_unregister(&shadow_type_1); + return ret; + } + ptr_id(NULL); /* @@ -296,12 +307,9 @@ static int test_klp_shadow_vars_init(void) pr_info(" got expected NULL result\n"); } - 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(); 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 = { .dtor = livepatch_fix1_dummy_leak_dtor, }; +struct klp_shadow_type *shadow_types[] = { + &shadow_leak_type, + NULL +}; + static struct klp_func funcs[] = { { .old_name = "dummy_alloc", @@ -156,6 +161,7 @@ static struct klp_object objs[] = { { .name = "livepatch_shadow_mod", .funcs = funcs, + .shadow_types = shadow_types, }, { } }; @@ -171,8 +177,6 @@ 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(&shadow_leak_type); } 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 = { .id = SV_COUNTER, }; +struct klp_shadow_type *shadow_types[] = { + &shadow_leak_type, + &shadow_counter_type, + NULL +}; + static struct klp_func funcs[] = { { .old_name = "dummy_check", @@ -118,6 +124,7 @@ static struct klp_object objs[] = { { .name = "livepatch_shadow_mod", .funcs = funcs, + .shadow_types = shadow_types, }, { } }; @@ -133,8 +140,6 @@ 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(&shadow_leak_type); } module_init(livepatch_shadow_fix2_init);