Message ID | f5ebbb87943bc583049909f4216ca1d73a794261.1686751371.git.steve@sakoman.com |
---|---|
State | New, archived |
Headers | show |
Series | [dunfell,1/5] libwebp: Fix CVE-2023-1999 | expand |
> -----Original Message----- > From: openembedded-core@lists.openembedded.org <openembedded- > core@lists.openembedded.org> On Behalf Of Steve Sakoman > Sent: den 14 juni 2023 16:05 > To: openembedded-core@lists.openembedded.org > Subject: [OE-core][dunfell 3/5] systemd-systemctl: fix instance template > WantedBy symlink construction > > From: Martin Siegumfeldt <mns@gomspace.com> > > Fix issue of the below instance template systemd service dependency > > [Install] > WantedBy=svc-wants@%i.service > > creating the symlink (instance "a" example) > > /etc/systemd/system/svc-wants@%i.service.wants/svc-wanted-by@a.service > > which should be > > /etc/systemd/system/svc-wants@a.service.wants/svc-wanted-by@a.service > > as implemented by this change. > > The functionality appears regressed just after "thud" baseline when the > logic was refactored from shell script into python (commit > 925e30cb104ece7bfa48b78144e758a46dc9ec3f) > > (From OE-Core rev: 308397f0bb3d6f3d4e9ec2c6a10823184049c9b5) > > Signed-off-by: Martin Siegumfeldt <mns@gomspace.com> > Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com> > Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org> > Signed-off-by: Steve Sakoman <steve@sakoman.com> > (cherry picked from commit 372b29c8ad270d4d430c26a4e614976c7029afaf) > Signed-off-by: Steve Sakoman <steve@sakoman.com> > --- > .../systemd/systemd-systemctl/systemctl | 13 ++++++++++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > > diff --git a/meta/recipes-core/systemd/systemd-systemctl/systemctl > b/meta/recipes-core/systemd/systemd-systemctl/systemctl > index 6aa2e20465..577c373181 100755 > --- a/meta/recipes-core/systemd/systemd-systemctl/systemctl > +++ b/meta/recipes-core/systemd/systemd-systemctl/systemctl > @@ -182,12 +182,19 @@ class SystemdUnit(): > > raise SystemdUnitNotFoundError(self.root, unit) > > - def _process_deps(self, config, service, location, prop, dirstem): > + def _process_deps(self, config, service, location, prop, dirstem, instance): > systemdir = self.root / SYSCONFDIR / "systemd" / "system" > > target = ROOT / location.relative_to(self.root) > try: > for dependent in config.get('Install', prop): > + # determine whether or not dependent is a template with an actual > + # instance (i.e. a '@%i') > + dependent_is_template = re.match(r"[^@]+@(?P<instance>[^\.]*)\.", dependent) > + if dependent_is_template: > + # if so, replace with the actual instance to achieve > + # svc-wants@a.service.wants/svc-wanted-by@a.service > + dependent = re.sub(dependent_is_template.group('instance'), instance, dependent, 1) > wants = systemdir / "{}.{}".format(dependent, dirstem) / service > add_link(wants, target) > > @@ -227,8 +234,8 @@ class SystemdUnit(): > else: > service = self.unit > > - self._process_deps(config, service, path, 'WantedBy', 'wants') > - self._process_deps(config, service, path, 'RequiredBy', 'requires') > + self._process_deps(config, service, path, 'WantedBy', 'wants', instance) > + self._process_deps(config, service, path, 'RequiredBy', 'requires', instance) > > try: > for also in config.get('Install', 'Also'): > -- > 2.34.1 You might want to be careful with this one. It broke systemctl for us when it hit Mickledore and we have had to backport the version from Langdale for now. The problem is that it does not handle non-instanced units depending on an instanced unit. In our case we have a unit foo.service that contains a RequiredBy=bar@0.service, which causes an error like this: Traceback (most recent call last): File ".../recipe-sysroot-native/usr/bin/systemctl", line 366, in <module> main() File ".../recipe-sysroot-native/usr/bin/systemctl", line 352, in main SystemdUnit(root, service).enable() File ".../recipe-sysroot-native/usr/bin/systemctl", line 250, in enable self._process_deps(config, service, path, 'WantedBy', 'wants', instance) File ".../recipe-sysroot-native/usr/bin/systemctl", line 210, in _process_deps dependent = re.sub(dependent_is_template.group('instance'), instance, dependent, 1) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File ".../recipe-sysroot-native/usr/lib/python3.11/re/__init__.py", line 185, in sub return _compile(pattern, flags).sub(repl, string, count) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File ".../recipe-sysroot-native/usr/lib/python3.11/re/__init__.py", line 317, in _subx template = _compile_repl(template, pattern) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File ".../recipe-sysroot-native/usr/lib/python3.11/re/__init__.py", line 308, in _compile_repl return _parser.parse_template(repl, pattern) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File ".../recipe-sysroot-native/usr/lib/python3.11/re/_parser.py", line 1000, in parse_template s = Tokenizer(source) ^^^^^^^^^^^^^^^^^ File ".../recipe-sysroot-native/usr/lib/python3.11/re/_parser.py", line 226, in __init__ string = str(string, 'latin1') ^^^^^^^^^^^^^^^^^^^^^ TypeError: decoding to str: need a bytes-like object, NoneType found There are probably other cases too when different kinds of units are involved (at least as far as I could gather from the systemd manual). We have an internal issue to fix this, but I am going on vacation on Monday, so I will unfortunately not have any time to look at it until after summer. //Peter
On Thu, 2023-06-15 at 20:40 +0000, Peter Kjellerstedt wrote: > > -----Original Message----- > > From: openembedded-core@lists.openembedded.org <openembedded- > > core@lists.openembedded.org> On Behalf Of Steve Sakoman > > Sent: den 14 juni 2023 16:05 > > To: openembedded-core@lists.openembedded.org > > Subject: [OE-core][dunfell 3/5] systemd-systemctl: fix instance > > template > > WantedBy symlink construction > > > > From: Martin Siegumfeldt <mns@gomspace.com> > > > > Fix issue of the below instance template systemd service dependency > > > > [Install] > > WantedBy=svc-wants@%i.service > > > > creating the symlink (instance "a" example) > > > > /etc/systemd/system/svc- > > wants@%i.service.wants/svc-wanted-by@a.service > > > > which should be > > > > /etc/systemd/system/svc-wants@a.service.wants > > /svc-wanted-by@a.service > > > > as implemented by this change. > > > > The functionality appears regressed just after "thud" baseline when > > the > > logic was refactored from shell script into python (commit > > 925e30cb104ece7bfa48b78144e758a46dc9ec3f) > > > > (From OE-Core rev: 308397f0bb3d6f3d4e9ec2c6a10823184049c9b5) > > > > Signed-off-by: Martin Siegumfeldt <mns@gomspace.com> > > Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com> > > Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org> > > Signed-off-by: Steve Sakoman <steve@sakoman.com> > > (cherry picked from commit > > 372b29c8ad270d4d430c26a4e614976c7029afaf) > > Signed-off-by: Steve Sakoman <steve@sakoman.com> > > --- > > .../systemd/systemd-systemctl/systemctl | 13 > > ++++++++++--- > > 1 file changed, 10 insertions(+), 3 deletions(-) > > > > diff --git a/meta/recipes-core/systemd/systemd-systemctl/systemctl > > b/meta/recipes-core/systemd/systemd-systemctl/systemctl > > index 6aa2e20465..577c373181 100755 > > --- a/meta/recipes-core/systemd/systemd-systemctl/systemctl > > +++ b/meta/recipes-core/systemd/systemd-systemctl/systemctl > > @@ -182,12 +182,19 @@ class SystemdUnit(): > > > > raise SystemdUnitNotFoundError(self.root, unit) > > > > - def _process_deps(self, config, service, location, prop, > > dirstem): > > + def _process_deps(self, config, service, location, prop, > > dirstem, instance): > > systemdir = self.root / SYSCONFDIR / "systemd" / "system" > > > > target = ROOT / location.relative_to(self.root) > > try: > > for dependent in config.get('Install', prop): > > + # determine whether or not dependent is a template > > with an actual > > + # instance (i.e. a '@%i') > > + dependent_is_template = > > re.match(r"[^@]+@(?P<instance>[^\.]*)\.", dependent) > > + if dependent_is_template: > > + # if so, replace with the actual instance to > > achieve > > + # > > svc-wants@a.service.wants/svc-wanted-by@a.service > > + dependent = > > re.sub(dependent_is_template.group('instance'), instance, > > dependent, 1) > > wants = systemdir / "{}.{}".format(dependent, > > dirstem) / service > > add_link(wants, target) > > > > @@ -227,8 +234,8 @@ class SystemdUnit(): > > else: > > service = self.unit > > > > - self._process_deps(config, service, path, 'WantedBy', > > 'wants') > > - self._process_deps(config, service, path, 'RequiredBy', > > 'requires') > > + self._process_deps(config, service, path, 'WantedBy', > > 'wants', instance) > > + self._process_deps(config, service, path, 'RequiredBy', > > 'requires', instance) > > > > try: > > for also in config.get('Install', 'Also'): > > -- > > 2.34.1 > > You might want to be careful with this one. It broke systemctl for us > when it > hit Mickledore and we have had to backport the version from Langdale > for now. > > The problem is that it does not handle non-instanced units depending > on > an instanced unit. In our case we have a unit foo.service that > contains > a RequiredBy=bar@0.service, which causes an error like this: > > Traceback (most recent call last): > File ".../recipe-sysroot-native/usr/bin/systemctl", line 366, in > <module> > main() > File ".../recipe-sysroot-native/usr/bin/systemctl", line 352, in > main > SystemdUnit(root, service).enable() > File ".../recipe-sysroot-native/usr/bin/systemctl", line 250, in > enable > self._process_deps(config, service, path, 'WantedBy', 'wants', > instance) > File ".../recipe-sysroot-native/usr/bin/systemctl", line 210, in > _process_deps > dependent = re.sub(dependent_is_template.group('instance'), > instance, dependent, 1) > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > ^^^^^^^^^^^^^^^^^^ > File ".../recipe-sysroot-native/usr/lib/python3.11/re/__init__.py", > line 185, in sub > return _compile(pattern, flags).sub(repl, string, count) > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > File ".../recipe-sysroot-native/usr/lib/python3.11/re/__init__.py", > line 317, in _subx > template = _compile_repl(template, pattern) > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > File ".../recipe-sysroot-native/usr/lib/python3.11/re/__init__.py", > line 308, in _compile_repl > return _parser.parse_template(repl, pattern) > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > File ".../recipe-sysroot-native/usr/lib/python3.11/re/_parser.py", > line 1000, in parse_template > s = Tokenizer(source) > ^^^^^^^^^^^^^^^^^ > File ".../recipe-sysroot-native/usr/lib/python3.11/re/_parser.py", > line 226, in __init__ > string = str(string, 'latin1') > ^^^^^^^^^^^^^^^^^^^^^ > TypeError: decoding to str: need a bytes-like object, NoneType found > > There are probably other cases too when different kinds of units are > involved (at least as far as I could gather from the systemd manual). > > We have an internal issue to fix this, but I am going on vacation on > Monday, so I will unfortunately not have any time to look at it until > after summer. There is a patch in master recently which may help? Cheers, Richard
On Thu, Jun 15, 2023 at 10:41 AM Peter Kjellerstedt <peter.kjellerstedt@axis.com> wrote: > > > -----Original Message----- > > From: openembedded-core@lists.openembedded.org <openembedded- > > core@lists.openembedded.org> On Behalf Of Steve Sakoman > > Sent: den 14 juni 2023 16:05 > > To: openembedded-core@lists.openembedded.org > > Subject: [OE-core][dunfell 3/5] systemd-systemctl: fix instance template > > WantedBy symlink construction > > > > From: Martin Siegumfeldt <mns@gomspace.com> > > > > Fix issue of the below instance template systemd service dependency > > > > [Install] > > WantedBy=svc-wants@%i.service > > > > creating the symlink (instance "a" example) > > > > /etc/systemd/system/svc-wants@%i.service.wants/svc-wanted-by@a.service > > > > which should be > > > > /etc/systemd/system/svc-wants@a.service.wants/svc-wanted-by@a.service > > > > as implemented by this change. > > > > The functionality appears regressed just after "thud" baseline when the > > logic was refactored from shell script into python (commit > > 925e30cb104ece7bfa48b78144e758a46dc9ec3f) > > > > (From OE-Core rev: 308397f0bb3d6f3d4e9ec2c6a10823184049c9b5) > > > > Signed-off-by: Martin Siegumfeldt <mns@gomspace.com> > > Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com> > > Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org> > > Signed-off-by: Steve Sakoman <steve@sakoman.com> > > (cherry picked from commit 372b29c8ad270d4d430c26a4e614976c7029afaf) > > Signed-off-by: Steve Sakoman <steve@sakoman.com> > > --- > > .../systemd/systemd-systemctl/systemctl | 13 ++++++++++--- > > 1 file changed, 10 insertions(+), 3 deletions(-) > > > > diff --git a/meta/recipes-core/systemd/systemd-systemctl/systemctl > > b/meta/recipes-core/systemd/systemd-systemctl/systemctl > > index 6aa2e20465..577c373181 100755 > > --- a/meta/recipes-core/systemd/systemd-systemctl/systemctl > > +++ b/meta/recipes-core/systemd/systemd-systemctl/systemctl > > @@ -182,12 +182,19 @@ class SystemdUnit(): > > > > raise SystemdUnitNotFoundError(self.root, unit) > > > > - def _process_deps(self, config, service, location, prop, dirstem): > > + def _process_deps(self, config, service, location, prop, dirstem, instance): > > systemdir = self.root / SYSCONFDIR / "systemd" / "system" > > > > target = ROOT / location.relative_to(self.root) > > try: > > for dependent in config.get('Install', prop): > > + # determine whether or not dependent is a template with an actual > > + # instance (i.e. a '@%i') > > + dependent_is_template = re.match(r"[^@]+@(?P<instance>[^\.]*)\.", dependent) > > + if dependent_is_template: > > + # if so, replace with the actual instance to achieve > > + # svc-wants@a.service.wants/svc-wanted-by@a.service > > + dependent = re.sub(dependent_is_template.group('instance'), instance, dependent, 1) > > wants = systemdir / "{}.{}".format(dependent, dirstem) / service > > add_link(wants, target) > > > > @@ -227,8 +234,8 @@ class SystemdUnit(): > > else: > > service = self.unit > > > > - self._process_deps(config, service, path, 'WantedBy', 'wants') > > - self._process_deps(config, service, path, 'RequiredBy', 'requires') > > + self._process_deps(config, service, path, 'WantedBy', 'wants', instance) > > + self._process_deps(config, service, path, 'RequiredBy', 'requires', instance) > > > > try: > > for also in config.get('Install', 'Also'): > > -- > > 2.34.1 > > You might want to be careful with this one. It broke systemctl for us when it > hit Mickledore and we have had to backport the version from Langdale for now. Thanks for reviewing. I'll drop this from dunfell for now and will look for the potential fix in master for mickledore. Steve > The problem is that it does not handle non-instanced units depending on > an instanced unit. In our case we have a unit foo.service that contains > a RequiredBy=bar@0.service, which causes an error like this: > > Traceback (most recent call last): > File ".../recipe-sysroot-native/usr/bin/systemctl", line 366, in <module> > main() > File ".../recipe-sysroot-native/usr/bin/systemctl", line 352, in main > SystemdUnit(root, service).enable() > File ".../recipe-sysroot-native/usr/bin/systemctl", line 250, in enable > self._process_deps(config, service, path, 'WantedBy', 'wants', instance) > File ".../recipe-sysroot-native/usr/bin/systemctl", line 210, in _process_deps > dependent = re.sub(dependent_is_template.group('instance'), instance, dependent, 1) > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > File ".../recipe-sysroot-native/usr/lib/python3.11/re/__init__.py", line 185, in sub > return _compile(pattern, flags).sub(repl, string, count) > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > File ".../recipe-sysroot-native/usr/lib/python3.11/re/__init__.py", line 317, in _subx > template = _compile_repl(template, pattern) > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > File ".../recipe-sysroot-native/usr/lib/python3.11/re/__init__.py", line 308, in _compile_repl > return _parser.parse_template(repl, pattern) > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > File ".../recipe-sysroot-native/usr/lib/python3.11/re/_parser.py", line 1000, in parse_template > s = Tokenizer(source) > ^^^^^^^^^^^^^^^^^ > File ".../recipe-sysroot-native/usr/lib/python3.11/re/_parser.py", line 226, in __init__ > string = str(string, 'latin1') > ^^^^^^^^^^^^^^^^^^^^^ > TypeError: decoding to str: need a bytes-like object, NoneType found > > There are probably other cases too when different kinds of units are > involved (at least as far as I could gather from the systemd manual). > > We have an internal issue to fix this, but I am going on vacation on > Monday, so I will unfortunately not have any time to look at it until > after summer. > > //Peter
diff --git a/meta/recipes-core/systemd/systemd-systemctl/systemctl b/meta/recipes-core/systemd/systemd-systemctl/systemctl index 6aa2e20465..577c373181 100755 --- a/meta/recipes-core/systemd/systemd-systemctl/systemctl +++ b/meta/recipes-core/systemd/systemd-systemctl/systemctl @@ -182,12 +182,19 @@ class SystemdUnit(): raise SystemdUnitNotFoundError(self.root, unit) - def _process_deps(self, config, service, location, prop, dirstem): + def _process_deps(self, config, service, location, prop, dirstem, instance): systemdir = self.root / SYSCONFDIR / "systemd" / "system" target = ROOT / location.relative_to(self.root) try: for dependent in config.get('Install', prop): + # determine whether or not dependent is a template with an actual + # instance (i.e. a '@%i') + dependent_is_template = re.match(r"[^@]+@(?P<instance>[^\.]*)\.", dependent) + if dependent_is_template: + # if so, replace with the actual instance to achieve + # svc-wants@a.service.wants/svc-wanted-by@a.service + dependent = re.sub(dependent_is_template.group('instance'), instance, dependent, 1) wants = systemdir / "{}.{}".format(dependent, dirstem) / service add_link(wants, target) @@ -227,8 +234,8 @@ class SystemdUnit(): else: service = self.unit - self._process_deps(config, service, path, 'WantedBy', 'wants') - self._process_deps(config, service, path, 'RequiredBy', 'requires') + self._process_deps(config, service, path, 'WantedBy', 'wants', instance) + self._process_deps(config, service, path, 'RequiredBy', 'requires', instance) try: for also in config.get('Install', 'Also'):