regression: 9a56493f6942 "uts: Use generic ns_common::count" broke makedumpfile 1.6.7

regression: 9a56493f6942 "uts: Use generic ns_common::count" broke makedumpfile 1.6.7

[Mike Galbraith]

Greetings,

With this commit, bisected and confirmed, kdump stops working here,
makedumpfile saying "check_release: Can't get the kernel version".

	-Mike

Re: regression: 9a56493f6942 "uts: Use generic ns_common::count" broke makedumpfile 1.6.7

[Kirill Tkhai]

Hi, Alexander,

On 16.12.2020 14:02, Mike Galbraith wrote:
> Greetings,
> 
> With this commit, bisected and confirmed, kdump stops working here,
> makedumpfile saying "check_release: Can't get the kernel version".

hasn't your commit 55d9e11398a4 "kdump: append uts_namespace.name offset to VMCOREINFO"
fixed this issue?

What problem with offset we meet here in case of uts_namespace is even marked with __randomize_layout?

Kirill

Re: regression: 9a56493f6942 "uts: Use generic ns_common::count" broke makedumpfile 1.6.7

[Mike Galbraith]

On Wed, 2020-12-16 at 15:35 +0300, Kirill Tkhai wrote:
> Hi, Alexander,
>
> On 16.12.2020 14:02, Mike Galbraith wrote:
> > Greetings,
> >
> > With this commit, bisected and confirmed, kdump stops working here,
> > makedumpfile saying "check_release: Can't get the kernel version".
>
> hasn't your commit 55d9e11398a4 "kdump: append uts_namespace.name offset to VMCOREINFO"
> fixed this issue?

FWIW, I applied the below, but it didn't help.

---
 kernel/crash_core.c |    1 +
 1 file changed, 1 insertion(+)

--- a/kernel/crash_core.c
+++ b/kernel/crash_core.c
@@ -447,6 +447,7 @@ static int __init crash_save_vmcoreinfo_
 	VMCOREINFO_PAGESIZE(PAGE_SIZE);

 	VMCOREINFO_SYMBOL(init_uts_ns);
+	VMCOREINFO_OFFSET(uts_namespace, name);
 	VMCOREINFO_SYMBOL(node_online_map);
 #ifdef CONFIG_MMU
 	VMCOREINFO_SYMBOL_ARRAY(swapper_pg_dir);

Re: regression: 9a56493f6942 "uts: Use generic ns_common::count" broke makedumpfile 1.6.7

[Kirill Tkhai]

On 16.12.2020 16:32, Mike Galbraith wrote:
> On Wed, 2020-12-16 at 15:35 +0300, Kirill Tkhai wrote:
>> Hi, Alexander,
>>
>> On 16.12.2020 14:02, Mike Galbraith wrote:
>>> Greetings,
>>>
>>> With this commit, bisected and confirmed, kdump stops working here,
>>> makedumpfile saying "check_release: Can't get the kernel version".
>>
>> hasn't your commit 55d9e11398a4 "kdump: append uts_namespace.name offset to VMCOREINFO"
>> fixed this issue?
> 
> FWIW, I applied the below, but it didn't help.
> 
> ---
>  kernel/crash_core.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> --- a/kernel/crash_core.c
> +++ b/kernel/crash_core.c
> @@ -447,6 +447,7 @@ static int __init crash_save_vmcoreinfo_
>  	VMCOREINFO_PAGESIZE(PAGE_SIZE);
> 
>  	VMCOREINFO_SYMBOL(init_uts_ns);
> +	VMCOREINFO_OFFSET(uts_namespace, name);
>  	VMCOREINFO_SYMBOL(node_online_map);
>  #ifdef CONFIG_MMU
>  	VMCOREINFO_SYMBOL_ARRAY(swapper_pg_dir);

As I see, makedumpfile hardcodes recent supported kernel version.
(I downloaded makedumpfile from here: https://github.com/makedumpfile/makedumpfile)

#define LATEST_VERSION          KERNEL_VERSION(5, 9, 4) /* linux-5.9.4 */
int32_t         
get_kernel_version(char *release)
{
	...
        if ((version < OLDEST_VERSION) || (LATEST_VERSION < version)) {
                MSG("The kernel version is not supported.\n");
                MSG("The makedumpfile operation may be incomplete.\n");
        }
	...
}

So, in case of you revert the patch, makedumpfile also should fail:

root@qemu:~# ./makedumpfile/makedumpfile -g VMCOREINFO -x ./vmlinux 
The kernel version is not supported.
The makedumpfile operation may be incomplete.

The vmcoreinfo is saved to VMCOREINFO.

makedumpfile Completed.

Does this regression only cause that one error message "check_release: Can't get the kernel version"
is printed instead of another: "The kernel version is not supported."?

Kirill

Re: regression: 9a56493f6942 "uts: Use generic ns_common::count" broke makedumpfile 1.6.7

[Christian Brauner]

On Wed, Dec 16, 2020 at 05:23:30PM +0300, Kirill Tkhai wrote:
> On 16.12.2020 16:32, Mike Galbraith wrote:
> > On Wed, 2020-12-16 at 15:35 +0300, Kirill Tkhai wrote:
> >> Hi, Alexander,
> >>
> >> On 16.12.2020 14:02, Mike Galbraith wrote:
> >>> Greetings,
> >>>
> >>> With this commit, bisected and confirmed, kdump stops working here,
> >>> makedumpfile saying "check_release: Can't get the kernel version".
> >>
> >> hasn't your commit 55d9e11398a4 "kdump: append uts_namespace.name offset to VMCOREINFO"
> >> fixed this issue?
> > 
> > FWIW, I applied the below, but it didn't help.
> > 
> > ---
> >  kernel/crash_core.c |    1 +
> >  1 file changed, 1 insertion(+)
> > 
> > --- a/kernel/crash_core.c
> > +++ b/kernel/crash_core.c
> > @@ -447,6 +447,7 @@ static int __init crash_save_vmcoreinfo_
> >  	VMCOREINFO_PAGESIZE(PAGE_SIZE);
> > 
> >  	VMCOREINFO_SYMBOL(init_uts_ns);
> > +	VMCOREINFO_OFFSET(uts_namespace, name);
> >  	VMCOREINFO_SYMBOL(node_online_map);
> >  #ifdef CONFIG_MMU
> >  	VMCOREINFO_SYMBOL_ARRAY(swapper_pg_dir);
> 
> As I see, makedumpfile hardcodes recent supported kernel version.
> (I downloaded makedumpfile from here: https://github.com/makedumpfile/makedumpfile)
> 
> #define LATEST_VERSION          KERNEL_VERSION(5, 9, 4) /* linux-5.9.4 */
> int32_t         
> get_kernel_version(char *release)
> {
> 	...
>         if ((version < OLDEST_VERSION) || (LATEST_VERSION < version)) {
>                 MSG("The kernel version is not supported.\n");
>                 MSG("The makedumpfile operation may be incomplete.\n");
>         }
> 	...
> }
> 
> So, in case of you revert the patch, makedumpfile also should fail:
> 
> root@qemu:~# ./makedumpfile/makedumpfile -g VMCOREINFO -x ./vmlinux 
> The kernel version is not supported.
> The makedumpfile operation may be incomplete.
> 
> The vmcoreinfo is saved to VMCOREINFO.
> 
> makedumpfile Completed.
> 
> Does this regression only cause that one error message "check_release: Can't get the kernel version"
> is printed instead of another: "The kernel version is not supported."?

(I take it you're debugging this now, Kirill. Thank you!)

Re: regression: 9a56493f6942 "uts: Use generic ns_common::count" broke makedumpfile 1.6.7

[Mike Galbraith]

On Wed, 2020-12-16 at 17:23 +0300, Kirill Tkhai wrote:
>
> Does this regression only cause that one error message "check_release: Can't get the kernel version"
> is printed instead of another: "The kernel version is not supported."?

The thing does indeed mutter about the kernel version, with or without
9a56493f6942 reverted, but it work despite the muttering with
9a56493f6942 reverted.

	-Mike

Re: regression: 9a56493f6942 "uts: Use generic ns_common::count" broke makedumpfile 1.6.7

[Mike Galbraith]

On Wed, 2020-12-16 at 15:31 +0100, Mike Galbraith wrote:
> On Wed, 2020-12-16 at 17:23 +0300, Kirill Tkhai wrote:
> >
> > Does this regression only cause that one error message "check_release: Can't get the kernel version"
> > is printed instead of another: "The kernel version is not supported."?
>
> The thing does indeed mutter about the kernel version, with or without
> 9a56493f6942 reverted, but it work despite the muttering with
> 9a56493f6942 reverted.

makedumpfile 1.6.7 source only claims to work up to linux-5.4.8, but
actually does work all the way up until 9a56493f6942.

If the answer here is that v1,6,7 has reached EOL, that's fine, I'll
just carry a revert until I can cobble together an updated package.

	-Mike

Re: regression: 9a56493f6942 "uts: Use generic ns_common::count" broke makedumpfile 1.6.7

[Kirill Tkhai]

On 16.12.2020 17:49, Mike Galbraith wrote:
> On Wed, 2020-12-16 at 15:31 +0100, Mike Galbraith wrote:
>> On Wed, 2020-12-16 at 17:23 +0300, Kirill Tkhai wrote:
>>>
>>> Does this regression only cause that one error message "check_release: Can't get the kernel version"
>>> is printed instead of another: "The kernel version is not supported."?
>>
>> The thing does indeed mutter about the kernel version, with or without
>> 9a56493f6942 reverted, but it work despite the muttering with
>> 9a56493f6942 reverted.
> 
> makedumpfile 1.6.7 source only claims to work up to linux-5.4.8, but
> actually does work all the way up until 9a56493f6942.
> 
> If the answer here is that v1,6,7 has reached EOL, that's fine, I'll
> just carry a revert until I can cobble together an updated package.

I think, since there is no a makedumpfile version working, this is a regression,
and it should be workarounded it in kernel.

We may consider a patch like the below till updated makedumpfile is not published:

diff --git a/include/linux/utsname.h b/include/linux/utsname.h
index 2b1737c9b244..d6bcad448f52 100644
--- a/include/linux/utsname.h
+++ b/include/linux/utsname.h
@@ -8,6 +8,7 @@
 #include <linux/ns_common.h>
 #include <linux/err.h>
 #include <uapi/linux/utsname.h>
+#include <linux/kref.h>
 
 enum uts_proc {
 	UTS_PROC_OSTYPE,
@@ -21,6 +22,7 @@ struct user_namespace;
 extern struct user_namespace init_user_ns;
 
 struct uts_namespace {
+	struct kref unused;
 	struct new_utsname name;
 	struct user_namespace *user_ns;
 	struct ucounts *ucounts;

Re: regression: 9a56493f6942 "uts: Use generic ns_common::count" broke makedumpfile 1.6.7

[Christian Brauner]

On Wed, Dec 16, 2020 at 06:20:36PM +0300, Kirill Tkhai wrote:
> On 16.12.2020 17:49, Mike Galbraith wrote:
> > On Wed, 2020-12-16 at 15:31 +0100, Mike Galbraith wrote:
> >> On Wed, 2020-12-16 at 17:23 +0300, Kirill Tkhai wrote:
> >>>
> >>> Does this regression only cause that one error message "check_release: Can't get the kernel version"
> >>> is printed instead of another: "The kernel version is not supported."?
> >>
> >> The thing does indeed mutter about the kernel version, with or without
> >> 9a56493f6942 reverted, but it work despite the muttering with
> >> 9a56493f6942 reverted.
> > 
> > makedumpfile 1.6.7 source only claims to work up to linux-5.4.8, but
> > actually does work all the way up until 9a56493f6942.
> > 
> > If the answer here is that v1,6,7 has reached EOL, that's fine, I'll
> > just carry a revert until I can cobble together an updated package.
> 
> I think, since there is no a makedumpfile version working, this is a regression,
> and it should be workarounded it in kernel.
> 
> We may consider a patch like the below till updated makedumpfile is not published:
> 
> diff --git a/include/linux/utsname.h b/include/linux/utsname.h
> index 2b1737c9b244..d6bcad448f52 100644
> --- a/include/linux/utsname.h
> +++ b/include/linux/utsname.h
> @@ -8,6 +8,7 @@
>  #include <linux/ns_common.h>
>  #include <linux/err.h>
>  #include <uapi/linux/utsname.h>
> +#include <linux/kref.h>
>  
>  enum uts_proc {
>  	UTS_PROC_OSTYPE,
> @@ -21,6 +22,7 @@ struct user_namespace;
>  extern struct user_namespace init_user_ns;
>  
>  struct uts_namespace {
> +	struct kref unused;
>  	struct new_utsname name;
>  	struct user_namespace *user_ns;
>  	struct ucounts *ucounts;

I'm not sure we really want to do that especially if the tool claims
that it's only supposed to work up to 5.4.8. But if we go down this
route then there should be a comment why this kref unused thing is
there.

Christian

Re: regression: 9a56493f6942 "uts: Use generic ns_common::count" broke makedumpfile 1.6.7

[Mike Galbraith]

On Wed, 2020-12-16 at 18:20 +0300, Kirill Tkhai wrote:
>
> We may consider a patch like the below till updated makedumpfile is not published:

It's nice to be considerate to the tool folks, but that can leave a
wake of cruft (thinks printk ringbuffer.. ew), so it's probably best to
just let it break.  The why and wherefore is documented in this thread,
that's enough for anybody who needs a quick fix to get on with frobbing
their dang buggy bleeding edge kernel ;-)

> diff --git a/include/linux/utsname.h b/include/linux/utsname.h
> index 2b1737c9b244..d6bcad448f52 100644
> --- a/include/linux/utsname.h
> +++ b/include/linux/utsname.h
> @@ -8,6 +8,7 @@
>  #include <linux/ns_common.h>
>  #include <linux/err.h>
>  #include <uapi/linux/utsname.h>
> +#include <linux/kref.h>
>
>  enum uts_proc {
>  	UTS_PROC_OSTYPE,
> @@ -21,6 +22,7 @@ struct user_namespace;
>  extern struct user_namespace init_user_ns;
>
>  struct uts_namespace {
> +	struct kref unused;
>  	struct new_utsname name;
>  	struct user_namespace *user_ns;
>  	struct ucounts *ucounts;

Re: regression: 9a56493f6942 "uts: Use generic ns_common::count" broke makedumpfile 1.6.7

[Alexander Egorenkov]

Kirill Tkhai <ktkhai@virtuozzo.com> writes:

> Hi, Alexander,
>
> On 16.12.2020 14:02, Mike Galbraith wrote:
>> Greetings,
>> 
>> With this commit, bisected and confirmed, kdump stops working here,
>> makedumpfile saying "check_release: Can't get the kernel version".
>
> hasn't your commit 55d9e11398a4 "kdump: append uts_namespace.name offset to VMCOREINFO"
> fixed this issue?
>
> What problem with offset we meet here in case of uts_namespace is even marked with __randomize_layout?
>
> Kirill

Hi Kirill,

the makedumpfile fix has been applied on Dec 17 2020.
makedumpfile complains about linux kernel version with newer ones but it should still work.

Regards
Alex