2022-10-19 09:03:19

by Torsten Hilbrich

[permalink] [raw]
Subject: v6.1-rc1: Regression in notification of sethostname changes

Hello Petr,

your commit

commit bfca3dd3d0680fc2fc7f659a152234afbac26e4d
Author: Petr Vorel <[email protected]>
Date: Thu Sep 1 21:44:03 2022 +0200

kernel/utsname_sysctl.c: print kernel arch

Print the machine hardware name (UTS_MACHINE) in /proc/sys/kernel/arch.

This helps people who debug kernel with initramfs with minimal environment
(i.e. without coreutils or even busybox) or allow to open sysfs file
instead of run 'uname -m' in high level languages.

broke the notification mechanism between the sethostname syscall and the pollers of /proc/sys/kernel/hostname.

The table uts_kern_table is addressed within uts_proc_notify by the enum value, however no new enum value was added in "enum uts_proc".

I noticed the problem when journald-systemd failed to detect hostname changes made with the sethostname syscall (as used by the hostname tool).
When setting the hostname through /proc/sys/kernel/hostname the poll notification was working.

Torsten


2022-10-19 11:11:27

by Petr Vorel

[permalink] [raw]
Subject: Re: v6.1-rc1: Regression in notification of sethostname changes

Hi Torsten,

> Hello Petr,

> your commit

> commit bfca3dd3d0680fc2fc7f659a152234afbac26e4d
> Author: Petr Vorel <[email protected]>
> Date: Thu Sep 1 21:44:03 2022 +0200

> kernel/utsname_sysctl.c: print kernel arch

> Print the machine hardware name (UTS_MACHINE) in /proc/sys/kernel/arch.

> This helps people who debug kernel with initramfs with minimal environment
> (i.e. without coreutils or even busybox) or allow to open sysfs file
> instead of run 'uname -m' in high level languages.

> broke the notification mechanism between the sethostname syscall and the pollers of /proc/sys/kernel/hostname.

> The table uts_kern_table is addressed within uts_proc_notify by the enum value, however no new enum value was added in "enum uts_proc".

> I noticed the problem when journald-systemd failed to detect hostname changes made with the sethostname syscall (as used by the hostname tool).
> When setting the hostname through /proc/sys/kernel/hostname the poll notification was working.

Thanks a lot for your report, working on a fix!
Andrew, Greg, sorry for a regression.

Kind regards,
Petr

> Torsten

2022-10-19 13:24:46

by Petr Vorel

[permalink] [raw]
Subject: Re: v6.1-rc1: Regression in notification of sethostname changes

> Hi Torsten,

> > Hello Petr,

> > your commit

> > commit bfca3dd3d0680fc2fc7f659a152234afbac26e4d
> > Author: Petr Vorel <[email protected]>
> > Date: Thu Sep 1 21:44:03 2022 +0200

> > kernel/utsname_sysctl.c: print kernel arch

> > Print the machine hardware name (UTS_MACHINE) in /proc/sys/kernel/arch.

> > This helps people who debug kernel with initramfs with minimal environment
> > (i.e. without coreutils or even busybox) or allow to open sysfs file
> > instead of run 'uname -m' in high level languages.

> > broke the notification mechanism between the sethostname syscall and the pollers of /proc/sys/kernel/hostname.

> > The table uts_kern_table is addressed within uts_proc_notify by the enum value, however no new enum value was added in "enum uts_proc".

> > I noticed the problem when journald-systemd failed to detect hostname changes made with the sethostname syscall (as used by the hostname tool).
> > When setting the hostname through /proc/sys/kernel/hostname the poll notification was working.

> Thanks a lot for your report, working on a fix!
> Andrew, Greg, sorry for a regression.

Hi Torsten,

could you please post exact steps to reproduce the problem.
Although the required fix to add new enum into enum uts_proc is trivial,
I was not able to reproduce the problem with 6.1.0-rc1 (actually
6.1.0-rc1-4.g1d716d8-default which contains few extra patches).

# hostname; hostnamectl hostname; cat /proc/sys/kernel/hostname
opensuse-tumbleweed.20221001
opensuse-tumbleweed.20221001
opensuse-tumbleweed.20221001

# hostnamectl set-hostname foo; echo $?
0
# hostname; hostnamectl hostname; cat /proc/sys/kernel/hostname
foo
foo
foo

# hostname bar; echo $?
0
# hostname; hostnamectl hostname; cat /proc/sys/kernel/hostname
bar
bar
bar

# echo "baz" > /proc/sys/kernel/hostname
# hostname; hostnamectl hostname; cat /proc/sys/kernel/hostname
baz
baz
baz


# hostnamectl set-hostname foo; reboot
After reboot it's 'foo'.
What am I missing?

BTW I originally tested the feature only on dracut initramfs (with rapido [1]),
which obviously bypass systemd. For a fix I'm creating rpm package (binrpm-pkg).

Kind regards,
Petr

[1] https://github.com/rapido-linux/rapido

> Kind regards,
> Petr

> > Torsten

2022-10-19 14:34:12

by Torsten Hilbrich

[permalink] [raw]
Subject: Re: v6.1-rc1: Regression in notification of sethostname changes

On 19.10.22 14:31, Petr Vorel wrote:
>> Hi Torsten,
>
>>> Hello Petr,
>
>>> your commit
>
>>> commit bfca3dd3d0680fc2fc7f659a152234afbac26e4d
>>> Author: Petr Vorel <[email protected]>
>>> Date: Thu Sep 1 21:44:03 2022 +0200
>
>>> kernel/utsname_sysctl.c: print kernel arch
>
>>> Print the machine hardware name (UTS_MACHINE) in /proc/sys/kernel/arch.
>
>>> This helps people who debug kernel with initramfs with minimal environment
>>> (i.e. without coreutils or even busybox) or allow to open sysfs file
>>> instead of run 'uname -m' in high level languages.
>
>>> broke the notification mechanism between the sethostname syscall and the pollers of /proc/sys/kernel/hostname.
>
>>> The table uts_kern_table is addressed within uts_proc_notify by the enum value, however no new enum value was added in "enum uts_proc".
>
>>> I noticed the problem when journald-systemd failed to detect hostname changes made with the sethostname syscall (as used by the hostname tool).
>>> When setting the hostname through /proc/sys/kernel/hostname the poll notification was working.
>
>> Thanks a lot for your report, working on a fix!
>> Andrew, Greg, sorry for a regression.
>
> Hi Torsten,
>
> could you please post exact steps to reproduce the problem.
> Although the required fix to add new enum into enum uts_proc is trivial,
> I was not able to reproduce the problem with 6.1.0-rc1 (actually
> 6.1.0-rc1-4.g1d716d8-default which contains few extra patches).
>
> # hostname; hostnamectl hostname; cat /proc/sys/kernel/hostname
> opensuse-tumbleweed.20221001
> opensuse-tumbleweed.20221001
> opensuse-tumbleweed.20221001
>
> # hostnamectl set-hostname foo; echo $?
> 0
> # hostname; hostnamectl hostname; cat /proc/sys/kernel/hostname
> foo
> foo
> foo
>
> # hostname bar; echo $?
> 0
> # hostname; hostnamectl hostname; cat /proc/sys/kernel/hostname
> bar
> bar
> bar
>
> # echo "baz" > /proc/sys/kernel/hostname
> # hostname; hostnamectl hostname; cat /proc/sys/kernel/hostname
> baz
> baz
> baz
>
>
> # hostnamectl set-hostname foo; reboot
> After reboot it's 'foo'.
> What am I missing?
>
> BTW I originally tested the feature only on dracut initramfs (with rapido [1]),
> which obviously bypass systemd. For a fix I'm creating rpm package (binrpm-pkg).

The problem is happening in the systemd-journald poll notification. I was checking for the problem by attaching gdb to the running systemd-journald and setting a breakpoint to the server_cache_hostname function. This function is triggered via dispatch_hostname_change whenever the hostname changes. This is done via sd_event API in systemd.

Here is an example program for this functionality without any further dependency:

#include <poll.h>
#include <fcntl.h>
#include <stdbool.h>
#include <stdio.h>
#include <unistd.h>

int main()
{
struct pollfd info;

info.fd = open("/proc/sys/kernel/hostname", O_RDONLY);
info.events = 0;
info.revents = 0;

while (true) {
int res = poll(&info, 1, -1);
if (res > 0) {
if (info.revents != 0) {
char buffer[64];
gethostname(buffer, sizeof(buffer));
printf("Hostname has changed to: %s\n", buffer);
}
}
}
}

I have also attached this program.

If you call this program and issue calls of the hostname utility to change the hostname some message should be printed.

Torsten


Attachments:
hostname-poll-test.c (474.00 B)

2022-10-20 10:38:50

by Petr Vorel

[permalink] [raw]
Subject: Re: v6.1-rc1: Regression in notification of sethostname changes

> On 19.10.22 14:31, Petr Vorel wrote:
> >> Hi Torsten,

> >>> Hello Petr,

> >>> your commit

> >>> commit bfca3dd3d0680fc2fc7f659a152234afbac26e4d
> >>> Author: Petr Vorel <[email protected]>
> >>> Date: Thu Sep 1 21:44:03 2022 +0200

> >>> kernel/utsname_sysctl.c: print kernel arch

> >>> Print the machine hardware name (UTS_MACHINE) in /proc/sys/kernel/arch.

> >>> This helps people who debug kernel with initramfs with minimal environment
> >>> (i.e. without coreutils or even busybox) or allow to open sysfs file
> >>> instead of run 'uname -m' in high level languages.

> >>> broke the notification mechanism between the sethostname syscall and the pollers of /proc/sys/kernel/hostname.

> >>> The table uts_kern_table is addressed within uts_proc_notify by the enum value, however no new enum value was added in "enum uts_proc".

> >>> I noticed the problem when journald-systemd failed to detect hostname changes made with the sethostname syscall (as used by the hostname tool).
> >>> When setting the hostname through /proc/sys/kernel/hostname the poll notification was working.

> >> Thanks a lot for your report, working on a fix!
> >> Andrew, Greg, sorry for a regression.

> > Hi Torsten,

> > could you please post exact steps to reproduce the problem.
> > Although the required fix to add new enum into enum uts_proc is trivial,
> > I was not able to reproduce the problem with 6.1.0-rc1 (actually
> > 6.1.0-rc1-4.g1d716d8-default which contains few extra patches).

> > # hostname; hostnamectl hostname; cat /proc/sys/kernel/hostname
> > opensuse-tumbleweed.20221001
> > opensuse-tumbleweed.20221001
> > opensuse-tumbleweed.20221001

> > # hostnamectl set-hostname foo; echo $?
> > 0
> > # hostname; hostnamectl hostname; cat /proc/sys/kernel/hostname
> > foo
> > foo
> > foo

> > # hostname bar; echo $?
> > 0
> > # hostname; hostnamectl hostname; cat /proc/sys/kernel/hostname
> > bar
> > bar
> > bar

> > # echo "baz" > /proc/sys/kernel/hostname
> > # hostname; hostnamectl hostname; cat /proc/sys/kernel/hostname
> > baz
> > baz
> > baz


> > # hostnamectl set-hostname foo; reboot
> > After reboot it's 'foo'.
> > What am I missing?

> > BTW I originally tested the feature only on dracut initramfs (with rapido [1]),
> > which obviously bypass systemd. For a fix I'm creating rpm package (binrpm-pkg).

> The problem is happening in the systemd-journald poll notification. I was checking for the problem by attaching gdb to the running systemd-journald and setting a breakpoint to the server_cache_hostname function. This function is triggered via dispatch_hostname_change whenever the hostname changes. This is done via sd_event API in systemd.

> Here is an example program for this functionality without any further dependency:

> #include <poll.h>
> #include <fcntl.h>
> #include <stdbool.h>
> #include <stdio.h>
> #include <unistd.h>

> int main()
> {
> struct pollfd info;

> info.fd = open("/proc/sys/kernel/hostname", O_RDONLY);
> info.events = 0;
> info.revents = 0;

> while (true) {
> int res = poll(&info, 1, -1);
> if (res > 0) {
> if (info.revents != 0) {
> char buffer[64];
> gethostname(buffer, sizeof(buffer));
> printf("Hostname has changed to: %s\n", buffer);
> }
> }
> }
> }

Rigth, poll() is broken. Thanks a lot for a simple reproducer!
I'll send a fix shortly.

Kind regards,
Petr

> I have also attached this program.

> If you call this program and issue calls of the hostname utility to change the hostname some message should be printed.

> Torsten


> #include <poll.h>
> #include <fcntl.h>
> #include <stdbool.h>
> #include <stdio.h>
> #include <unistd.h>

> int main()
> {
> struct pollfd info;

> info.fd = open("/proc/sys/kernel/hostname", O_RDONLY);
> info.events = 0;
> info.revents = 0;

> while (true) {
> int res = poll(&info, 1, -1);
> if (res > 0) {
> if (info.revents != 0) {
> char buffer[64];
> gethostname(buffer, sizeof(buffer));
> printf("Hostname has changed to: %s\n", buffer);
> }
> }
> }
> }

Subject: Re: v6.1-rc1: Regression in notification of sethostname changes

[TLDR: I'm adding this regression report to the list of tracked
regressions; all text from me you find below is based on a few templates
paragraphs you might have encountered already already in similar form.]

Hi, this is your Linux kernel regression tracker. CCing the regression
mailing list, as it should be in the loop for all regressions, as
explained here:
https://www.kernel.org/doc/html/latest/admin-guide/reporting-issues.html



On 19.10.22 10:29, Torsten Hilbrich wrote:
> Hello Petr,
>
> your commit
>
> commit bfca3dd3d0680fc2fc7f659a152234afbac26e4d
> Author: Petr Vorel <[email protected]>
> Date: Thu Sep 1 21:44:03 2022 +0200
>
> kernel/utsname_sysctl.c: print kernel arch
>
> Print the machine hardware name (UTS_MACHINE) in /proc/sys/kernel/arch.
>
> This helps people who debug kernel with initramfs with minimal environment
> (i.e. without coreutils or even busybox) or allow to open sysfs file
> instead of run 'uname -m' in high level languages.
>
> broke the notification mechanism between the sethostname syscall and the pollers of /proc/sys/kernel/hostname.
>
> The table uts_kern_table is addressed within uts_proc_notify by the enum value, however no new enum value was added in "enum uts_proc".
>
> I noticed the problem when journald-systemd failed to detect hostname changes made with the sethostname syscall (as used by the hostname tool).
> When setting the hostname through /proc/sys/kernel/hostname the poll notification was working.

Thanks for the report. To be sure below issue doesn't fall through the
cracks unnoticed, I'm adding it to regzbot, my Linux kernel regression
tracking bot:

#regzbot ^introduced bfca3dd3d0
#regzbot ignore-activity

This isn't a regression? This issue or a fix for it are already
discussed somewhere else? It was fixed already? You want to clarify when
the regression started to happen? Or point out I got the title or
something else totally wrong? Then just reply -- ideally with also
telling regzbot about it, as explained here:
https://linux-regtracking.leemhuis.info/tracked-regression/

Reminder for developers: When fixing the issue, add 'Link:' tags
pointing to the report (the mail this one replies to), as explained for
in the Linux kernel's documentation; above webpage explains why this is
important for tracked regressions.

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)

P.S.: As the Linux kernel's regression tracker I deal with a lot of
reports and sometimes miss something important when writing mails like
this. If that's the case here, don't hesitate to tell me in a public
reply, it's in everyone's interest to set the public record straight.