Live-Patching Archive on lore.kernel.org
 help / color / Atom feed
From: Yannick Cote <ycote@redhat.com>
To: live-patching@vger.kernel.org
Cc: linux-kselftest@vger.kernel.org, joe.lawrence@redhat.com
Subject: [PATCH 2/4] selftests/livepatch: rework test-klp-shadow-vars
Date: Thu, 28 May 2020 09:48:47 -0400
Message-ID: <20200528134849.7890-3-ycote@redhat.com> (raw)
In-Reply-To: <20200528134849.7890-1-ycote@redhat.com>

The initial idea was to make a change to please cppcheck and remove void
pointer arithmetics found a few times:

	portability: 'obj' is of type 'void *'. When using void pointers
	             in calculations, the behaviour is undefined.
		     [arithOperationsOnVoidPointer]

The rest of the changes are to help make the test read as an example
while continuing to verify the shadow variable code. The logic of the
test is unchanged but restructured to use descriptive names.

Signed-off-by: Yannick Cote <ycote@redhat.com>
---
 lib/livepatch/test_klp_shadow_vars.c | 101 +++++++++++++++------------
 1 file changed, 57 insertions(+), 44 deletions(-)

diff --git a/lib/livepatch/test_klp_shadow_vars.c b/lib/livepatch/test_klp_shadow_vars.c
index f0b5a1d24e55..ec2635cff974 100644
--- a/lib/livepatch/test_klp_shadow_vars.c
+++ b/lib/livepatch/test_klp_shadow_vars.c
@@ -109,8 +109,7 @@ static void shadow_free(void *obj, unsigned long id, klp_shadow_dtor_t dtor)
 static void shadow_free_all(unsigned long id, klp_shadow_dtor_t dtor)
 {
 	klp_shadow_free_all(id, dtor);
-	pr_info("klp_%s(id=0x%lx, dtor=PTR%d)\n",
-		__func__, id, ptr_id(dtor));
+	pr_info("klp_%s(id=0x%lx, dtor=PTR%d)\n", __func__, id, ptr_id(dtor));
 }
 
 
@@ -124,8 +123,7 @@ static int shadow_ctor(void *obj, void *shadow_data, void *ctor_data)
 		return -EINVAL;
 
 	*sv = *var;
-	pr_info("%s: PTR%d -> PTR%d\n",
-		__func__, ptr_id(sv), ptr_id(*var));
+	pr_info("%s: PTR%d -> PTR%d\n", __func__, ptr_id(sv), ptr_id(*var));
 
 	return 0;
 }
@@ -138,49 +136,63 @@ static void shadow_dtor(void *obj, void *shadow_data)
 		__func__, ptr_id(obj), ptr_id(sv));
 }
 
-static int test_klp_shadow_vars_init(void)
-{
-	void *obj			= THIS_MODULE;
-	int id			= 0x1234;
-	gfp_t gfp_flags		= GFP_KERNEL;
+/* dynamically created obj fields have the following shadow var id values */
+#define SV_ID1 0x1234
+#define SV_ID2 0x1235
 
-	int var1, var2, var3, var4;
-	int *pv1, *pv2, *pv3, *pv4;
-	int **sv1, **sv2, **sv3, **sv4;
+/*
+ * 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
+ * the idea that shadow variables may be added and removed to and from the
+ * struct during execution.
+ */
+struct test_object {
+	 /* add anything here below and avoid to define an empty struct */
+	struct shadow_ptr sp;
 
-	int **sv;
+	/* these represent shadow vars added and removed with SV_ID{1,2} */
+	/* char nfield1; */
+	/* int  nfield2; */
+};
 
-	pv1 = &var1;
-	pv2 = &var2;
-	pv3 = &var3;
-	pv4 = &var4;
+static int test_klp_shadow_vars_init(void)
+{
+	struct test_object obj1, obj2, obj3;
+	char nfield1, nfield2, *pnfield1, *pnfield2, **sv1, **sv2;
+	int  nfield3, nfield4, *pnfield3, *pnfield4, **sv3, **sv4;
+	void **sv;
+
+	pnfield1 = &nfield1;
+	pnfield2 = &nfield2;
+	pnfield3 = &nfield3;
+	pnfield4 = &nfield4;
 
 	ptr_id(NULL);
-	ptr_id(pv1);
-	ptr_id(pv2);
-	ptr_id(pv3);
-	ptr_id(pv4);
+	ptr_id(pnfield1);
+	ptr_id(pnfield2);
+	ptr_id(pnfield3);
+	ptr_id(pnfield4);
 
 	/*
 	 * With an empty shadow variable hash table, expect not to find
 	 * any matches.
 	 */
-	sv = shadow_get(obj, id);
+	sv = shadow_get(&obj1, SV_ID1);
 	if (!sv)
 		pr_info("  got expected NULL result\n");
 
 	/*
 	 * Allocate a few shadow variables with different <obj> and <id>.
 	 */
-	sv1 = shadow_alloc(obj, id, sizeof(pv1), gfp_flags, shadow_ctor, &pv1);
+	sv1 = shadow_alloc(&obj1, SV_ID1, sizeof(pnfield1), GFP_KERNEL, shadow_ctor, &pnfield1);
 	if (!sv1)
 		return -ENOMEM;
 
-	sv2 = shadow_alloc(obj + 1, id, sizeof(pv2), gfp_flags, shadow_ctor, &pv2);
+	sv2 = shadow_alloc(&obj2, SV_ID1, sizeof(pnfield2), GFP_KERNEL, shadow_ctor, &pnfield2);
 	if (!sv2)
 		return -ENOMEM;
 
-	sv3 = shadow_alloc(obj, id + 1, sizeof(pv3), gfp_flags, shadow_ctor, &pv3);
+	sv3 = shadow_alloc(&obj1, SV_ID2, sizeof(pnfield3), GFP_KERNEL, shadow_ctor, &pnfield3);
 	if (!sv3)
 		return -ENOMEM;
 
@@ -188,23 +200,24 @@ static int test_klp_shadow_vars_init(void)
 	 * Verify we can find our new shadow variables and that they point
 	 * to expected data.
 	 */
-	sv = shadow_get(obj, id);
+	sv = shadow_get(&obj1, SV_ID1);
 	if (!sv)
 		return -EINVAL;
-	if (sv == sv1 && *sv1 == pv1)
+	if ((char **)sv == sv1 && *sv1 == pnfield1)
 		pr_info("  got expected PTR%d -> PTR%d result\n",
 			ptr_id(sv1), ptr_id(*sv1));
 
-	sv = shadow_get(obj + 1, id);
+	sv = shadow_get(&obj2, SV_ID1);
 	if (!sv)
 		return -EINVAL;
-	if (sv == sv2 && *sv2 == pv2)
+	if ((char **)sv == sv2 && *sv2 == pnfield2)
 		pr_info("  got expected PTR%d -> PTR%d result\n",
 			ptr_id(sv2), ptr_id(*sv2));
-	sv = shadow_get(obj, id + 1);
+
+	sv = shadow_get(&obj1, SV_ID2);
 	if (!sv)
 		return -EINVAL;
-	if (sv == sv3 && *sv3 == pv3)
+	if ((int **)sv == sv3 && *sv3 == pnfield3)
 		pr_info("  got expected PTR%d -> PTR%d result\n",
 			ptr_id(sv3), ptr_id(*sv3));
 
@@ -212,14 +225,14 @@ static int test_klp_shadow_vars_init(void)
 	 * Allocate or get a few more, this time with the same <obj>, <id>.
 	 * The second invocation should return the same shadow var.
 	 */
-	sv4 = shadow_get_or_alloc(obj + 2, id, sizeof(pv4), gfp_flags, shadow_ctor, &pv4);
+	sv4 = shadow_get_or_alloc(&obj3, SV_ID1, sizeof(pnfield4), GFP_KERNEL, shadow_ctor, &pnfield4);
 	if (!sv4)
 		return -ENOMEM;
 
-	sv = shadow_get_or_alloc(obj + 2, id, sizeof(pv4), gfp_flags, shadow_ctor, &pv4);
+	sv = shadow_get_or_alloc(&obj3, SV_ID1, sizeof(pnfield4), GFP_KERNEL, shadow_ctor, &pnfield4);
 	if (!sv)
 		return -EINVAL;
-	if (sv == sv4 && *sv4 == pv4)
+	if ((int **)sv == sv4 && *sv4 == pnfield4)
 		pr_info("  got expected PTR%d -> PTR%d result\n",
 			ptr_id(sv4), ptr_id(*sv4));
 
@@ -227,36 +240,36 @@ static int test_klp_shadow_vars_init(void)
 	 * Free the <obj=*, id> shadow variables and check that we can no
 	 * longer find them.
 	 */
-	shadow_free(obj, id, shadow_dtor);			/* sv1 */
-	sv = shadow_get(obj, id);
+	shadow_free(&obj1, SV_ID1, shadow_dtor);		/* sv1 */
+	sv = shadow_get(&obj1, SV_ID1);
 	if (!sv)
 		pr_info("  got expected NULL result\n");
 
-	shadow_free(obj + 1, id, shadow_dtor);			/* sv2 */
-	sv = shadow_get(obj + 1, id);
+	shadow_free(&obj2, SV_ID1, shadow_dtor);		/* sv2 */
+	sv = shadow_get(&obj2, SV_ID1);
 	if (!sv)
 		pr_info("  got expected NULL result\n");
 
-	shadow_free(obj + 2, id, shadow_dtor);			/* sv4 */
-	sv = shadow_get(obj + 2, id);
+	shadow_free(&obj3, SV_ID1, shadow_dtor);		/* sv4 */
+	sv = shadow_get(&obj3, SV_ID1);
 	if (!sv)
 		pr_info("  got expected NULL result\n");
 
 	/*
 	 * We should still find an <id+1> variable.
 	 */
-	sv = shadow_get(obj, id + 1);
+	sv = shadow_get(&obj1, SV_ID2);
 	if (!sv)
 		return -EINVAL;
-	if (sv == sv3 && *sv3 == pv3)
+	if ((int **)sv == sv3 && *sv3 == pnfield3)
 		pr_info("  got expected PTR%d -> PTR%d result\n",
 			ptr_id(sv3), ptr_id(*sv3));
 
 	/*
 	 * Free all the <id+1> variables, too.
 	 */
-	shadow_free_all(id + 1, shadow_dtor);			/* sv3 */
-	sv = shadow_get(obj, id);
+	shadow_free_all(SV_ID2, shadow_dtor);			/* sv3 */
+	sv = shadow_get(&obj1, SV_ID1);
 	if (!sv)
 		pr_info("  shadow_get() got expected NULL result\n");
 
-- 
2.25.4


  parent reply index

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-28 13:48 [PATCH 0/4] selftests/livepatch: rework of test-klp-{callbacks,shadow_vars} Yannick Cote
2020-05-28 13:48 ` [PATCH 1/4] selftests/livepatch: rework test-klp-callbacks to use completion variables Yannick Cote
2020-06-02  8:16   ` Petr Mladek
2020-05-28 13:48 ` Yannick Cote [this message]
2020-06-02  9:25   ` [PATCH 2/4] selftests/livepatch: rework test-klp-shadow-vars Petr Mladek
2020-05-28 13:48 ` [PATCH 3/4] selftests/livepatch: more verification in test-klp-shadow-vars Yannick Cote
2020-06-01 11:27   ` Miroslav Benes
2020-06-01 11:39   ` Miroslav Benes
2020-06-02 12:35   ` Petr Mladek
2020-05-28 13:48 ` [PATCH 4/4] selftests/livepatch: fix mem leaks " Yannick Cote
2020-06-02  9:57   ` Petr Mladek
2020-05-29 15:12 ` [PATCH 0/4] selftests/livepatch: rework of test-klp-{callbacks,shadow_vars} Joe Lawrence
2020-06-01 11:48 ` Miroslav Benes
2020-06-02  5:01 ` Kamalesh Babulal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200528134849.7890-3-ycote@redhat.com \
    --to=ycote@redhat.com \
    --cc=joe.lawrence@redhat.com \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=live-patching@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Live-Patching Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/live-patching/0 live-patching/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 live-patching live-patching/ https://lore.kernel.org/live-patching \
		live-patching@vger.kernel.org
	public-inbox-index live-patching

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.live-patching


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git