[PATCH] livepatch: create-diff-object: Check that the section has a secsym

[PATCH] livepatch: create-diff-object: Check that the section has a secsym

[Sarah Newman]

A STT_SECTION symbol is not needed if if it is not used as a relocation
target. Therefore, a section, in this case a debug section, may not have
a secsym associated with it.

This is a livepatch backport of kpatch upstream commit [1]:
create-diff-object: Check that the section has a secsym ba3defa

Signed-off-by: Sarah Newman <sarah.newman@computer.org>

[1] https://github.com/dynup/kpatch/commit/ba3defa06073dcc69917dbbbb9df570ca4e56612
---
 create-diff-object.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/create-diff-object.c b/create-diff-object.c
index a516670..780e6c8 100644
--- a/create-diff-object.c
+++ b/create-diff-object.c
@@ -1484,7 +1484,7 @@ static void kpatch_include_debug_sections(struct kpatch_elf *kelf)
 	list_for_each_entry(sec, &kelf->sections, list) {
 		if (is_debug_section(sec)) {
 			sec->include = 1;
-			if (!is_rela_section(sec))
+			if (!is_rela_section(sec) && sec->secsym)
 				sec->secsym->include = 1;
 		}
 	}
-- 
2.17.1

Re: [PATCH] livepatch: create-diff-object: Check that the section has a secsym

[Jan Beulich]

On 06.07.2022 04:18, Sarah Newman wrote:
> A STT_SECTION symbol is not needed if if it is not used as a relocation
> target. Therefore, a section, in this case a debug section, may not have
> a secsym associated with it.
> 
> This is a livepatch backport of kpatch upstream commit [1]:
> create-diff-object: Check that the section has a secsym ba3defa
> 
> Signed-off-by: Sarah Newman <sarah.newman@computer.org>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

However I think the 7-digit hash above wants either extending to at
least 10 digits (better 12), or dropping altogether for having the
full hash ...

> [1] https://github.com/dynup/kpatch/commit/ba3defa06073dcc69917dbbbb9df570ca4e56612

... here anyway. But I guess the rules set forth in
xen.git:docs/process/sending-patches.pandoc could be applied here,
and hence an Origin: tag would best be used.

The other thing is: You didn't Cc any of the maintainer(s).

Jan

Re: [PATCH] livepatch: create-diff-object: Check that the section has a secsym

[Ross Lagerwall]

> From: Xen-devel <xen-devel-bounces@lists.xenproject.org> on behalf of Sarah Newman <sarah.newman@computer.org>
> Sent: Wednesday, July 6, 2022 3:18 AM
> To: Xen-devel@lists.xenproject.org <Xen-devel@lists.xenproject.org>
> Cc: Sarah Newman <sarah.newman@computer.org>
> Subject: [PATCH] livepatch: create-diff-object: Check that the section has a secsym 
>  
> A STT_SECTION symbol is not needed if if it is not used as a relocation
> target. Therefore, a section, in this case a debug section, may not have
> a secsym associated with it.
> 
> This is a livepatch backport of kpatch upstream commit [1]:
> create-diff-object: Check that the section has a secsym ba3defa
> 
> Signed-off-by: Sarah Newman <sarah.newman@computer.org>
> 

Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>

[PATCH v2] livepatch: create-diff-object: Check that the section has a secsym

[Sarah Newman]

A STT_SECTION symbol is not needed if if it is not used as a relocation
target. Therefore, a section, in this case a debug section, may not have
a secsym associated with it.

Origin: https://github.com/dynup/kpatch.git ba3defa06073
Signed-off-by: Sarah Newman <sarah.newman@computer.org>
---
Changes in v2:
- commit message changed to use Origin
---
 create-diff-object.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/create-diff-object.c b/create-diff-object.c
index a516670..780e6c8 100644
--- a/create-diff-object.c
+++ b/create-diff-object.c
@@ -1484,7 +1484,7 @@ static void kpatch_include_debug_sections(struct kpatch_elf *kelf)
 	list_for_each_entry(sec, &kelf->sections, list) {
 		if (is_debug_section(sec)) {
 			sec->include = 1;
-			if (!is_rela_section(sec))
+			if (!is_rela_section(sec) && sec->secsym)
 				sec->secsym->include = 1;
 		}
 	}
-- 
2.17.1

Re: [PATCH v2] livepatch: create-diff-object: Check that the section has a secsym

[Jan Beulich]

On 25.07.2022 05:20, Sarah Newman wrote:
> A STT_SECTION symbol is not needed if if it is not used as a relocation
> target. Therefore, a section, in this case a debug section, may not have
> a secsym associated with it.
> 
> Origin: https://github.com/dynup/kpatch.git ba3defa06073
> Signed-off-by: Sarah Newman <sarah.newman@computer.org>
> ---
> Changes in v2:
> - commit message changed to use Origin

With this I don't see why you didn't keep Ross'es R-b. Please help
committers by keeping tags up-to-date.

Jan

[PATCH v3] livepatch: create-diff-object: Check that the section has a secsym

[Sarah Newman]

A STT_SECTION symbol is not needed if if it is not used as a relocation
target. Therefore, a section, in this case a debug section, may not have
a secsym associated with it.

Signed-off-by: Bill Wendling <morbo@google.com>
Origin: https://github.com/dynup/kpatch.git ba3defa06073
Signed-off-by: Sarah Newman <sarah.newman@computer.org>
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
---
Changes in v3:
- add reviewed-by given to v1 of this patch
- restored tag from original commit per sending-patches.pandoc
Changes in v2:
- commit message changed to use Origin
---
 create-diff-object.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/create-diff-object.c b/create-diff-object.c
index a516670..780e6c8 100644
--- a/create-diff-object.c
+++ b/create-diff-object.c
@@ -1484,7 +1484,7 @@ static void kpatch_include_debug_sections(struct kpatch_elf *kelf)
 	list_for_each_entry(sec, &kelf->sections, list) {
 		if (is_debug_section(sec)) {
 			sec->include = 1;
-			if (!is_rela_section(sec))
+			if (!is_rela_section(sec) && sec->secsym)
 				sec->secsym->include = 1;
 		}
 	}
-- 
2.17.1

Re: [PATCH v3] livepatch: create-diff-object: Check that the section has a secsym

[Jan Beulich]

On 25.07.2022 19:13, Sarah Newman wrote:
> A STT_SECTION symbol is not needed if if it is not used as a relocation
> target. Therefore, a section, in this case a debug section, may not have
> a secsym associated with it.
> 
> Signed-off-by: Bill Wendling <morbo@google.com>

Hmm - this wasn't here before. Does this then suggest the patch also
wants to be marked From: Bill?

> Origin: https://github.com/dynup/kpatch.git ba3defa06073
> Signed-off-by: Sarah Newman <sarah.newman@computer.org>
> Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>

Sigh. I had given R-b on v1 as well. Actually I had meant to commit this
yesterday (with all adjustments made), but as it turns out committers
can't commit to that tree. So it'll be up to Ross or Konrad to actually
take care of this.

Jan

Re: [PATCH v3] livepatch: create-diff-object: Check that the section has a secsym

[Sarah Newman]

On 7/25/22 23:25, Jan Beulich wrote:
> On 25.07.2022 19:13, Sarah Newman wrote:
>> A STT_SECTION symbol is not needed if if it is not used as a relocation
>> target. Therefore, a section, in this case a debug section, may not have
>> a secsym associated with it.
>>
>> Signed-off-by: Bill Wendling <morbo@google.com>
> 
> Hmm - this wasn't here before. Does this then suggest the patch also
> wants to be marked From: Bill?

I don't know what the etiquette is here since this was a commit originally committed to kpatch, I just added that back because the xen patch 
submission guidelines said to preserve original tags.

> 
>> Origin: https://github.com/dynup/kpatch.git ba3defa06073
>> Signed-off-by: Sarah Newman <sarah.newman@computer.org>
>> Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
> 
> Sigh. I had given R-b on v1 as well. Actually I had meant to commit this
> yesterday (with all adjustments made), but as it turns out committers
> can't commit to that tree. So it'll be up to Ross or Konrad to actually
> take care of this.
> 
> Jan
> 

My apologies, I simply missed that.

I am doing this in my spare time. This is the first time I've gone through this process in a couple of years and have only done it a few times total.

Thanks, Sarah