Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1268113 > unrolled thread

[PATCH 3/4 v5] livepatch: add sympos as disambiguator field to klp_reloc

Started byChris J Arges <chris.j.arges@canonical.com>
First post2015-11-12 18:10 +0100
Last post2015-11-13 17:50 +0100
Articles 3 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 3/4 v5] livepatch: add sympos as disambiguator field to klp_reloc Chris J Arges <chris.j.arges@canonical.com> - 2015-11-12 18:10 +0100
    Re: [PATCH 3/4 v5] livepatch: add sympos as disambiguator field to  klp_reloc Josh Poimboeuf <jpoimboe@redhat.com> - 2015-11-12 21:00 +0100
    Re: [PATCH 3/4 v5] livepatch: add sympos as disambiguator field to  klp_reloc Petr Mladek <pmladek@suse.com> - 2015-11-13 17:50 +0100

#1268113 — [PATCH 3/4 v5] livepatch: add sympos as disambiguator field to klp_reloc

FromChris J Arges <chris.j.arges@canonical.com>
Date2015-11-12 18:10 +0100
Subject[PATCH 3/4 v5] livepatch: add sympos as disambiguator field to klp_reloc
Message-ID<qu3xL-3gW-9@gated-at.bofh.it>
In cases of duplicate symbols, sympos will be used to disambiguate instead
of val. By default sympos will be 0, and patching will only succeed if
the symbol is unique. Specifying a positive value will ensure that
occurrence of the symbol in kallsyms for the patched object will be used
for patching if it is valid.

Remove klp_verify_callback, klp_verify_args and klp_verify_vmlinux_symbol
as they are no longer used.

From the klp_reloc structure remove val, as it can be refactored as a
local variable in klp_write_object_relocations.

Signed-off-by: Chris J Arges <chris.j.arges@canonical.com>
---
 include/linux/livepatch.h |  5 ++-
 kernel/livepatch/core.c   | 81 +++++++++++++----------------------------------
 2 files changed, 24 insertions(+), 62 deletions(-)

diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
index 9808dd4..9149de4 100644
--- a/include/linux/livepatch.h
+++ b/include/linux/livepatch.h
@@ -68,8 +68,7 @@ struct klp_func {
 /**
  * struct klp_reloc - relocation structure for live patching
  * @loc:	address where the relocation will be written
- * @val:	address of the referenced symbol (optional,
- *		vmlinux	patches only)
+ * @sympos:	position in kallsyms to disambiguate symbols (optional)
  * @type:	ELF relocation type
  * @name:	name of the referenced symbol (for lookup/verification)
  * @addend:	offset from the referenced symbol
@@ -78,7 +77,7 @@ struct klp_func {
  */
 struct klp_reloc {
 	unsigned long loc;
-	unsigned long val;
+	unsigned long sympos;
 	unsigned long type;
 	const char *name;
 	int addend;
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index 4cd079c..071895f 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -202,45 +202,6 @@ static int klp_find_object_symbol(const char *objname, const char *name,
 	return -EINVAL;
 }
 
-struct klp_verify_args {
-	const char *name;
-	const unsigned long addr;
-};
-
-static int klp_verify_callback(void *data, const char *name,
-			       struct module *mod, unsigned long addr)
-{
-	struct klp_verify_args *args = data;
-
-	if (!mod &&
-	    !strcmp(args->name, name) &&
-	    args->addr == addr)
-		return 1;
-
-	return 0;
-}
-
-static int klp_verify_vmlinux_symbol(const char *name, unsigned long addr)
-{
-	struct klp_verify_args args = {
-		.name = name,
-		.addr = addr,
-	};
-	int ret;
-
-	mutex_lock(&module_mutex);
-	ret = kallsyms_on_each_symbol(klp_verify_callback, &args);
-	mutex_unlock(&module_mutex);
-
-	if (!ret) {
-		pr_err("symbol '%s' not found at specified address 0x%016lx, kernel mismatch?\n",
-			name, addr);
-		return -EINVAL;
-	}
-
-	return 0;
-}
-
 /*
  * External symbols are exported symbols that are defined outside both
  * the patched object and the patch.
@@ -266,6 +227,7 @@ static int klp_write_object_relocations(struct module *pmod,
 					struct klp_object *obj)
 {
 	int ret;
+	unsigned long val;
 	struct klp_reloc *reloc;
 
 	if (WARN_ON(!klp_is_object_loaded(obj)))
@@ -275,29 +237,30 @@ static int klp_write_object_relocations(struct module *pmod,
 		return -EINVAL;
 
 	for (reloc = obj->relocs; reloc->name; reloc++) {
-		if (!klp_is_module(obj)) {
-			ret = klp_verify_vmlinux_symbol(reloc->name,
-							reloc->val);
-			if (ret)
-				return ret;
-		} else {
-			/* module, reloc->val needs to be discovered */
-			if (reloc->external)
-				ret = klp_find_external_symbol(pmod,
-							       reloc->name,
-							       &reloc->val);
-			else
-				ret = klp_find_object_symbol(obj->mod->name,
-							     reloc->name,
-							     &reloc->val, 0);
-			if (ret)
-				return ret;
-		}
+		/* discover the address of the referenced symbol */
+		if (reloc->external) {
+			/*
+			 * external symbols aren't specified by the user
+			 * therefore sympos should not be used
+			 */
+			if (reloc->sympos) {
+				pr_err("symbol '%s' is external and has sympos %lu\n",
+				       reloc->name, reloc->sympos);
+				return -EINVAL;
+			}
+			ret = klp_find_external_symbol(pmod, reloc->name, &val);
+		} else
+			ret = klp_find_object_symbol(obj->mod->name,
+						     reloc->name,
+						     &val,
+						     reloc->sympos);
+		if (ret)
+			return ret;
 		ret = klp_write_module_reloc(pmod, reloc->type, reloc->loc,
-					     reloc->val + reloc->addend);
+					     val + reloc->addend);
 		if (ret) {
 			pr_err("relocation failed for symbol '%s' at 0x%016lx (%d)\n",
-			       reloc->name, reloc->val, ret);
+			       reloc->name, val, ret);
 			return ret;
 		}
 	}
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1268238 — Re: [PATCH 3/4 v5] livepatch: add sympos as disambiguator field to klp_reloc

FromJosh Poimboeuf <jpoimboe@redhat.com>
Date2015-11-12 21:00 +0100
SubjectRe: [PATCH 3/4 v5] livepatch: add sympos as disambiguator field to klp_reloc
Message-ID<qu6ch-4LN-11@gated-at.bofh.it>
In reply to#1268113
On Thu, Nov 12, 2015 at 10:59:52AM -0600, Chris J Arges wrote:
> In cases of duplicate symbols, sympos will be used to disambiguate instead
> of val. By default sympos will be 0, and patching will only succeed if
> the symbol is unique. Specifying a positive value will ensure that
> occurrence of the symbol in kallsyms for the patched object will be used
> for patching if it is valid.
> 
> Remove klp_verify_callback, klp_verify_args and klp_verify_vmlinux_symbol
> as they are no longer used.
> 
> From the klp_reloc structure remove val, as it can be refactored as a
> local variable in klp_write_object_relocations.
> 
> Signed-off-by: Chris J Arges <chris.j.arges@canonical.com>
> ---
>  include/linux/livepatch.h |  5 ++-
>  kernel/livepatch/core.c   | 81 +++++++++++++----------------------------------
>  2 files changed, 24 insertions(+), 62 deletions(-)
> 
> diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
> index 9808dd4..9149de4 100644
> --- a/include/linux/livepatch.h
> +++ b/include/linux/livepatch.h
> @@ -68,8 +68,7 @@ struct klp_func {
>  /**
>   * struct klp_reloc - relocation structure for live patching
>   * @loc:	address where the relocation will be written
> - * @val:	address of the referenced symbol (optional,
> - *		vmlinux	patches only)
> + * @sympos:	position in kallsyms to disambiguate symbols (optional)
>   * @type:	ELF relocation type
>   * @name:	name of the referenced symbol (for lookup/verification)
>   * @addend:	offset from the referenced symbol
> @@ -78,7 +77,7 @@ struct klp_func {
>   */
>  struct klp_reloc {
>  	unsigned long loc;
> -	unsigned long val;
> +	unsigned long sympos;
>  	unsigned long type;
>  	const char *name;
>  	int addend;
> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> index 4cd079c..071895f 100644
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
> @@ -202,45 +202,6 @@ static int klp_find_object_symbol(const char *objname, const char *name,
>  	return -EINVAL;
>  }
>  
> -struct klp_verify_args {
> -	const char *name;
> -	const unsigned long addr;
> -};
> -
> -static int klp_verify_callback(void *data, const char *name,
> -			       struct module *mod, unsigned long addr)
> -{
> -	struct klp_verify_args *args = data;
> -
> -	if (!mod &&
> -	    !strcmp(args->name, name) &&
> -	    args->addr == addr)
> -		return 1;
> -
> -	return 0;
> -}
> -
> -static int klp_verify_vmlinux_symbol(const char *name, unsigned long addr)
> -{
> -	struct klp_verify_args args = {
> -		.name = name,
> -		.addr = addr,
> -	};
> -	int ret;
> -
> -	mutex_lock(&module_mutex);
> -	ret = kallsyms_on_each_symbol(klp_verify_callback, &args);
> -	mutex_unlock(&module_mutex);
> -
> -	if (!ret) {
> -		pr_err("symbol '%s' not found at specified address 0x%016lx, kernel mismatch?\n",
> -			name, addr);
> -		return -EINVAL;
> -	}
> -
> -	return 0;
> -}
> -
>  /*
>   * External symbols are exported symbols that are defined outside both
>   * the patched object and the patch.
> @@ -266,6 +227,7 @@ static int klp_write_object_relocations(struct module *pmod,
>  					struct klp_object *obj)
>  {
>  	int ret;
> +	unsigned long val;
>  	struct klp_reloc *reloc;
>  
>  	if (WARN_ON(!klp_is_object_loaded(obj)))
> @@ -275,29 +237,30 @@ static int klp_write_object_relocations(struct module *pmod,
>  		return -EINVAL;
>  
>  	for (reloc = obj->relocs; reloc->name; reloc++) {
> -		if (!klp_is_module(obj)) {
> -			ret = klp_verify_vmlinux_symbol(reloc->name,
> -							reloc->val);
> -			if (ret)
> -				return ret;
> -		} else {
> -			/* module, reloc->val needs to be discovered */
> -			if (reloc->external)
> -				ret = klp_find_external_symbol(pmod,
> -							       reloc->name,
> -							       &reloc->val);
> -			else
> -				ret = klp_find_object_symbol(obj->mod->name,
> -							     reloc->name,
> -							     &reloc->val, 0);
> -			if (ret)
> -				return ret;
> -		}
> +		/* discover the address of the referenced symbol */
> +		if (reloc->external) {
> +			/*
> +			 * external symbols aren't specified by the user
> +			 * therefore sympos should not be used
> +			 */

"external symbols' *objects* aren't specified by the user..."

> +			if (reloc->sympos) {
> +				pr_err("symbol '%s' is external and has sympos %lu\n",
> +				       reloc->name, reloc->sympos);
> +				return -EINVAL;
> +			}
> +			ret = klp_find_external_symbol(pmod, reloc->name, &val);
> +		} else
> +			ret = klp_find_object_symbol(obj->mod->name,
> +						     reloc->name,
> +						     &val,
> +						     reloc->sympos);
> +		if (ret)
> +			return ret;
>  		ret = klp_write_module_reloc(pmod, reloc->type, reloc->loc,
> -					     reloc->val + reloc->addend);
> +					     val + reloc->addend);
>  		if (ret) {
>  			pr_err("relocation failed for symbol '%s' at 0x%016lx (%d)\n",
> -			       reloc->name, reloc->val, ret);
> +			       reloc->name, val, ret);
>  			return ret;
>  		}
>  	}
> -- 
> 1.9.1
> 

-- 
Josh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1269080 — Re: [PATCH 3/4 v5] livepatch: add sympos as disambiguator field to klp_reloc

FromPetr Mladek <pmladek@suse.com>
Date2015-11-13 17:50 +0100
SubjectRe: [PATCH 3/4 v5] livepatch: add sympos as disambiguator field to klp_reloc
Message-ID<qupHY-qL-5@gated-at.bofh.it>
In reply to#1268113
On Thu 2015-11-12 10:59:52, Chris J Arges wrote:
> In cases of duplicate symbols, sympos will be used to disambiguate instead
> of val. By default sympos will be 0, and patching will only succeed if
> the symbol is unique. Specifying a positive value will ensure that
> occurrence of the symbol in kallsyms for the patched object will be used
> for patching if it is valid.
> 
> Remove klp_verify_callback, klp_verify_args and klp_verify_vmlinux_symbol
> as they are no longer used.
> 
> >From the klp_reloc structure remove val, as it can be refactored as a
> local variable in klp_write_object_relocations.
> 
> Signed-off-by: Chris J Arges <chris.j.arges@canonical.com>
> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> index 4cd079c..071895f 100644
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
> @@ -275,29 +237,30 @@ static int klp_write_object_relocations(struct module *pmod,
>  		return -EINVAL;
>  
>  	for (reloc = obj->relocs; reloc->name; reloc++) {
> -		if (!klp_is_module(obj)) {
> -			ret = klp_verify_vmlinux_symbol(reloc->name,
> -							reloc->val);
> -			if (ret)
> -				return ret;
> -		} else {
> -			/* module, reloc->val needs to be discovered */
> -			if (reloc->external)
> -				ret = klp_find_external_symbol(pmod,
> -							       reloc->name,
> -							       &reloc->val);
> -			else
> -				ret = klp_find_object_symbol(obj->mod->name,
> -							     reloc->name,
> -							     &reloc->val, 0);
> -			if (ret)
> -				return ret;
> -		}
> +		/* discover the address of the referenced symbol */
> +		if (reloc->external) {
> +			/*
> +			 * external symbols aren't specified by the user
> +			 * therefore sympos should not be used
> +			 */
> +			if (reloc->sympos) {
> +				pr_err("symbol '%s' is external and has sympos %lu\n",
> +				       reloc->name, reloc->sympos);
> +				return -EINVAL;
> +			}

To be precise, reloc->sympos == 1 is a valid value. Note that it will show
"symbol,1" in sysfs. I suggest the following to keep it simple:

			if (reloc->sympos && (reloc->sympos != 1)) {
				pr_err("relocated external symbols must be unique but '%s' has sympos %lu\n",
				       reloc->name, reloc->sympos);
				return -EINVAL;
			}

Then we could even get rid of the comment.

Huh, I am looking forward to get rid of the external symbols ;-)

Thank you,
Petr
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web