Re: [systemd-devel] Significant performance loss caused by commit a65f06b: journal: return -ECHILD after a fork

[Zbigniew Jędrzejewski-Szmek]

On Fri, Jul 07, 2017 at 03:54:09PM -0700, vcap...@pengaru.com wrote:
> On Fri, Jul 07, 2017 at 10:34:22PM +, Zbigniew Jędrzejewski-Szmek wrote:
> > On Fri, Jul 07, 2017 at 02:35:16PM -0700, vcap...@pengaru.com wrote:
> > > On Fri, Jul 07, 2017 at 01:49:54PM -0700, vcap...@pengaru.com wrote:
> > > > On Fri, Jul 07, 2017 at 08:37:08PM +, Mantas Mikulėnas wrote:
> > > > > Back when that commit was made, didn't glibc cache the getpid() 
> > > > > result in
> > > > > userspace? That would explain why it was not noticed.
> > > >
> > > > Hmm, this crossed my mind, and come to think of it I did a dist-upgrade
> > > > from Debian jessie to stretch overnight machine and haven't rebooted.
> > > > 
> > > > Perhaps the vdso isn't working and the costly getpid() is a red 
> > > > herring, will
> > > > reboot and retest to confirm.
> > > > 
> > > 
> > > It appears Debian has a glibc patch to disable the caching (I was unaware
> > > such an elaborate dance was being performed to cache this!)
> > > 
> > > https://anonscm.debian.org/cgit/pkg-glibc/glibc.git/commit/debian/patches/any?id=5850253f509604dd46a6131acc057ea26e1588ba
> > 
> > Do we know the justification for this patch?
> > 
> 
> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=857909
> 
> Which references this upstream glibc bug:
> 
> https://sourceware.org/bugzilla/show_bug.cgi?id=19957
> https://sourceware.org/git/gitweb.cgi?p=glibc.git;a=commit;h=0cb313f7cb0e418b3d56f3a2ac69790522ab825d
> 
> 
> > > Unsure where I stand on core system software assuming certain syscalls are
> > > always going to be exceptionally cheap though...
> > 
> > Optimization is never in a vacuum. If glibc does something cheaply, it
> > seems reasonable to take advantage of it.
> > 
> 
> Except there's always a risk of these things regressing to normal syscalls,
> and one has to weigh the utility against that.  It's unclear to me what
> significant utility having the sd-journal API police changing pids by
> calling getpid() at every public entrypoint is bringing to the table.

So it seems the issue has been fixed in glibc upstream more than a year
ago, and it doesn't seem to make sense to optimize current systemd git for
that.

I see the argument that the getpid() checks are a bit excessive. Is their
overhead actually noticeable with current glibc?

Zbyszek

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Github systemd issue 6237

[Reindl Harald]

Am 07.07.2017 um 21:55 schrieb Kai Krakow:

Am Tue, 4 Jul 2017 21:23:01 + (UTC)
schrieb Alexander Bisogiannis :


On Tue, 04 Jul 2017 17:21:01 +, Zbigniew Jędrzejewski-Szmek wrote:


If you need root permissions to create a unit, then it's not a
security issue. An annoyance at most.


The fact that you need to be root to create a unit file is irrelevant.

Systemd is running a service as a different user to what is defined
in the unit file.
This is a bug and a local security issue, especially because it will
run said service as root.

It might not warrant a CVE, although in my line of work this is
considered a security issue, but it is a bug and needs fixing.

The fix is to refuse to run the service, period.


There's nothing to fix because it already works that way: If you give
it a valid user name that does not exists, the system refuses to start
the unit with "user not found"


and if you give a invalid username it has to do the same - PERIOD

systemd is directly after the kernel the most important and lowest level 
stuff on a setup and hence can't be handeled like some random stuff

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Github systemd issue 6237

[Michael Chapman]

On Sat, 8 Jul 2017, Kai Krakow wrote:
[...]

The bug here is that a leading number will "convert" to the number and
it actually runs with the UID specified that way: 0day = 0, 7days = 7.


No, this is not the case. Only all-digit User= values are treated as UIDs.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Significant performance loss caused by commit a65f06b: journal: return -ECHILD after a fork

[vcaputo]

On Fri, Jul 07, 2017 at 10:34:22PM +, Zbigniew Jędrzejewski-Szmek wrote:
> On Fri, Jul 07, 2017 at 02:35:16PM -0700, vcap...@pengaru.com wrote:
> > On Fri, Jul 07, 2017 at 01:49:54PM -0700, vcap...@pengaru.com wrote:
> > > On Fri, Jul 07, 2017 at 08:37:08PM +, Mantas Mikulėnas wrote:
> > > > Back when that commit was made, didn't glibc cache the getpid() result 
> > > > in
> > > > userspace? That would explain why it was not noticed.
> > >
> > > Hmm, this crossed my mind, and come to think of it I did a dist-upgrade
> > > from Debian jessie to stretch overnight machine and haven't rebooted.
> > > 
> > > Perhaps the vdso isn't working and the costly getpid() is a red herring, 
> > > will
> > > reboot and retest to confirm.
> > > 
> > 
> > It appears Debian has a glibc patch to disable the caching (I was unaware
> > such an elaborate dance was being performed to cache this!)
> > 
> > https://anonscm.debian.org/cgit/pkg-glibc/glibc.git/commit/debian/patches/any?id=5850253f509604dd46a6131acc057ea26e1588ba
> 
> Do we know the justification for this patch?
> 

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=857909

Which references this upstream glibc bug:

https://sourceware.org/bugzilla/show_bug.cgi?id=19957
https://sourceware.org/git/gitweb.cgi?p=glibc.git;a=commit;h=0cb313f7cb0e418b3d56f3a2ac69790522ab825d


> > Unsure where I stand on core system software assuming certain syscalls are
> > always going to be exceptionally cheap though...
> 
> Optimization is never in a vacuum. If glibc does something cheaply, it
> seems reasonable to take advantage of it.
> 

Except there's always a risk of these things regressing to normal syscalls,
and one has to weigh the utility against that.  It's unclear to me what
significant utility having the sd-journal API police changing pids by
calling getpid() at every public entrypoint is bringing to the table.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Significant performance loss caused by commit a65f06b: journal: return -ECHILD after a fork

[Zbigniew Jędrzejewski-Szmek]

On Fri, Jul 07, 2017 at 02:35:16PM -0700, vcap...@pengaru.com wrote:
> On Fri, Jul 07, 2017 at 01:49:54PM -0700, vcap...@pengaru.com wrote:
> > On Fri, Jul 07, 2017 at 08:37:08PM +, Mantas Mikulėnas wrote:
> > > Back when that commit was made, didn't glibc cache the getpid() result in
> > > userspace? That would explain why it was not noticed.
> >
> > Hmm, this crossed my mind, and come to think of it I did a dist-upgrade
> > from Debian jessie to stretch overnight machine and haven't rebooted.
> > 
> > Perhaps the vdso isn't working and the costly getpid() is a red herring, 
> > will
> > reboot and retest to confirm.
> > 
> 
> It appears Debian has a glibc patch to disable the caching (I was unaware
> such an elaborate dance was being performed to cache this!)
> 
> https://anonscm.debian.org/cgit/pkg-glibc/glibc.git/commit/debian/patches/any?id=5850253f509604dd46a6131acc057ea26e1588ba

Do we know the justification for this patch?

> Unsure where I stand on core system software assuming certain syscalls are
> always going to be exceptionally cheap though...

Optimization is never in a vacuum. If glibc does something cheaply, it
seems reasonable to take advantage of it.

Zbyszek
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Significant performance loss caused by commit a65f06b: journal: return -ECHILD after a fork

[vcaputo]

On Fri, Jul 07, 2017 at 01:49:54PM -0700, vcap...@pengaru.com wrote:
> On Fri, Jul 07, 2017 at 08:37:08PM +, Mantas Mikulėnas wrote:
> > Back when that commit was made, didn't glibc cache the getpid() result in
> > userspace? That would explain why it was not noticed.
>
> Hmm, this crossed my mind, and come to think of it I did a dist-upgrade
> from Debian jessie to stretch overnight machine and haven't rebooted.
> 
> Perhaps the vdso isn't working and the costly getpid() is a red herring, will
> reboot and retest to confirm.
> 

It appears Debian has a glibc patch to disable the caching (I was unaware
such an elaborate dance was being performed to cache this!)

https://anonscm.debian.org/cgit/pkg-glibc/glibc.git/commit/debian/patches/any?id=5850253f509604dd46a6131acc057ea26e1588ba

Unsure where I stand on core system software assuming certain syscalls are
always going to be exceptionally cheap though...
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Significant performance loss caused by commit a65f06b: journal: return -ECHILD after a fork

[vcaputo]

On Fri, Jul 07, 2017 at 01:49:54PM -0700, vcap...@pengaru.com wrote:
> On Fri, Jul 07, 2017 at 08:37:08PM +, Mantas Mikulėnas wrote:
> > Back when that commit was made, didn't glibc cache the getpid() result in
> > userspace? That would explain why it was not noticed.
>
> Hmm, this crossed my mind, and come to think of it I did a dist-upgrade
> from Debian jessie to stretch overnight machine and haven't rebooted.
> 
> Perhaps the vdso isn't working and the costly getpid() is a red herring, will
> reboot and retest to confirm.

Nope, same getpid() performance hit after rebooting.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Significant performance loss caused by commit a65f06b: journal: return -ECHILD after a fork

[Mantas Mikulėnas]

Back when that commit was made, didn't glibc cache the getpid() result in
userspace? That would explain why it was not noticed.

On Fri, Jul 7, 2017, 23:18  wrote:

> In doing some casual journalctl profiling and stracing, it became apparent
> that `journalctl -b --no-pager` runs across a significant quantity of logs,
> ~10% of the time was thrown away on getpid() calls due to commmit a65f06b.
>
> As-is:
>  # time ./journalctl -b --no-pager > /dev/null
>
>  real0m11.033s
>  user0m10.084s
>  sys 0m0.943s
>
>
> After changing journal_pid_changed() to simply return 1:
>  # time ./journalctl -b --no-pager > /dev/null
>
>   real0m9.641s
>   user0m9.449s
>   sys 0m0.191s
>
>
> More system time is being expended in repeated getpid() calls than write(),
> see the strace:
>
>  12:51:56.939287 write(1, "Jul 07 09:25:23 x61s unknown"..., 57) = 57
> <0.001276>
>  12:51:56.940633 getpid()= 10713 <0.32>
>  12:51:56.940732 getpid()= 10713 <0.12>
>  12:51:56.940801 getpid()= 10713 <0.32>
>  12:51:56.940867 getpid()= 10713 <0.41>
>  12:51:56.940942 getpid()= 10713 <0.41>
>  12:51:56.941047 getpid()= 10713 <0.12>
>  12:51:56.941117 getpid()= 10713 <0.12>
>  12:51:56.941185 getpid()= 10713 <0.11>
>  12:51:56.941253 getpid()= 10713 <0.11>
>  12:51:56.941320 getpid()= 10713 <0.39>
>  12:51:56.941395 getpid()= 10713 <0.41>
>  12:51:56.941494 getpid()= 10713 <0.11>
>  12:51:56.941561 getpid()= 10713 <0.12>
>  12:51:56.941629 getpid()= 10713 <0.39>
>  12:51:56.942942 write(1, "Jul 07 09:25:23 x61s unknown"..., 57) = 57
> <0.58>
>  12:51:56.943052 getpid()= 10713 <0.39>
>  12:51:56.943156 getpid()= 10713 <0.17>
>  12:51:56.943230 getpid()= 10713 <0.18>
>  12:51:56.943305 getpid()= 10713 <0.12>
>  12:51:56.943374 getpid()= 10713 <0.17>
>  12:51:56.943449 getpid()= 10713 <0.11>
>  12:51:56.943517 getpid()= 10713 <0.11>
>  12:51:56.943585 getpid()= 10713 <0.11>
>  12:51:56.943652 getpid()= 10713 <0.11>
>  12:51:56.943721 getpid()= 10713 <0.30>
>  12:51:56.943796 getpid()= 10713 <0.41>
>  12:51:56.943870 getpid()= 10713 <0.41>
>  12:51:56.943944 getpid()= 10713 <0.41>
>  12:51:56.944061 getpid()= 10713 <0.001334>
>  12:51:56.945459 write(1, "Jul 07 09:25:23 x61s unknown"..., 56) = 56
> <0.18>
>  12:51:56.945544 getpid()= 10713 <0.17>
>  12:51:56.945620 getpid()= 10713 <0.17>
>  12:51:56.945694 getpid()= 10713 <0.12>
>  12:51:56.945763 getpid()= 10713 <0.11>
>  12:51:56.945832 getpid()= 10713 <0.12>
>  12:51:56.945901 getpid()= 10713 <0.11>
>  12:51:56.945969 getpid()= 10713 <0.11>
>  12:51:56.946048 getpid()= 10713 <0.13>
>  12:51:56.946118 getpid()= 10713 <0.24>
>  12:51:56.946188 getpid()= 10713 <0.47>
>  12:51:56.946277 getpid()= 10713 <0.41>
>  12:51:56.946353 getpid()= 10713 <0.41>
>  12:51:56.946428 getpid()= 10713 <0.40>
>  12:51:56.946539 getpid()= 10713 <0.001363>
>
> As this is public sd-journal API, it's somewhat set in stone.  However,
> there's nothing preventing the systemd-internal tooling from linking with
> a less defensive/conformant underlying implementation shared with the
> public
> API implementation where these kinds of overheads can be preserved.
>
> For the curious; the logs being processed for this boot are 48 * 8MiB on
> SSD,
> 1.8Ghz Core2 Duo, 4.12 kernel.
>
> Regards,
> Vito Caputo
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/systemd-devel
>
-- 

Mantas Mikulėnas 
Sent from my phone
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Significant performance loss caused by commit a65f06b: journal: return -ECHILD after a fork

[vcaputo]

Hmm, this crossed my mind, and come to think of it I did a dist-upgrade
from Debian jessie to stretch overnight machine and haven't rebooted.

Perhaps the vdso isn't working and the costly getpid() is a red herring, will
reboot and retest to confirm.


On Fri, Jul 07, 2017 at 08:37:08PM +, Mantas Mikulėnas wrote:
> Back when that commit was made, didn't glibc cache the getpid() result in
> userspace? That would explain why it was not noticed.
> 
> On Fri, Jul 7, 2017, 23:18  wrote:
> 
> > In doing some casual journalctl profiling and stracing, it became apparent
> > that `journalctl -b --no-pager` runs across a significant quantity of logs,
> > ~10% of the time was thrown away on getpid() calls due to commmit a65f06b.
> >
> > As-is:
> >  # time ./journalctl -b --no-pager > /dev/null
> >
> >  real0m11.033s
> >  user0m10.084s
> >  sys 0m0.943s
> >
> >
> > After changing journal_pid_changed() to simply return 1:
> >  # time ./journalctl -b --no-pager > /dev/null
> >
> >   real0m9.641s
> >   user0m9.449s
> >   sys 0m0.191s
> >
> >
> > More system time is being expended in repeated getpid() calls than write(),
> > see the strace:
> >
> >  12:51:56.939287 write(1, "Jul 07 09:25:23 x61s unknown"..., 57) = 57
> > <0.001276>
> >  12:51:56.940633 getpid()= 10713 <0.32>
> >  12:51:56.940732 getpid()= 10713 <0.12>
> >  12:51:56.940801 getpid()= 10713 <0.32>
> >  12:51:56.940867 getpid()= 10713 <0.41>
> >  12:51:56.940942 getpid()= 10713 <0.41>
> >  12:51:56.941047 getpid()= 10713 <0.12>
> >  12:51:56.941117 getpid()= 10713 <0.12>
> >  12:51:56.941185 getpid()= 10713 <0.11>
> >  12:51:56.941253 getpid()= 10713 <0.11>
> >  12:51:56.941320 getpid()= 10713 <0.39>
> >  12:51:56.941395 getpid()= 10713 <0.41>
> >  12:51:56.941494 getpid()= 10713 <0.11>
> >  12:51:56.941561 getpid()= 10713 <0.12>
> >  12:51:56.941629 getpid()= 10713 <0.39>
> >  12:51:56.942942 write(1, "Jul 07 09:25:23 x61s unknown"..., 57) = 57
> > <0.58>
> >  12:51:56.943052 getpid()= 10713 <0.39>
> >  12:51:56.943156 getpid()= 10713 <0.17>
> >  12:51:56.943230 getpid()= 10713 <0.18>
> >  12:51:56.943305 getpid()= 10713 <0.12>
> >  12:51:56.943374 getpid()= 10713 <0.17>
> >  12:51:56.943449 getpid()= 10713 <0.11>
> >  12:51:56.943517 getpid()= 10713 <0.11>
> >  12:51:56.943585 getpid()= 10713 <0.11>
> >  12:51:56.943652 getpid()= 10713 <0.11>
> >  12:51:56.943721 getpid()= 10713 <0.30>
> >  12:51:56.943796 getpid()= 10713 <0.41>
> >  12:51:56.943870 getpid()= 10713 <0.41>
> >  12:51:56.943944 getpid()= 10713 <0.41>
> >  12:51:56.944061 getpid()= 10713 <0.001334>
> >  12:51:56.945459 write(1, "Jul 07 09:25:23 x61s unknown"..., 56) = 56
> > <0.18>
> >  12:51:56.945544 getpid()= 10713 <0.17>
> >  12:51:56.945620 getpid()= 10713 <0.17>
> >  12:51:56.945694 getpid()= 10713 <0.12>
> >  12:51:56.945763 getpid()= 10713 <0.11>
> >  12:51:56.945832 getpid()= 10713 <0.12>
> >  12:51:56.945901 getpid()= 10713 <0.11>
> >  12:51:56.945969 getpid()= 10713 <0.11>
> >  12:51:56.946048 getpid()= 10713 <0.13>
> >  12:51:56.946118 getpid()= 10713 <0.24>
> >  12:51:56.946188 getpid()= 10713 <0.47>
> >  12:51:56.946277 getpid()= 10713 <0.41>
> >  12:51:56.946353 getpid()= 10713 <0.41>
> >  12:51:56.946428 getpid()= 10713 <0.40>
> >  12:51:56.946539 getpid()= 10713 <0.001363>
> >
> > As this is public sd-journal API, it's somewhat set in stone.  However,
> > there's nothing preventing the systemd-internal tooling from linking with
> > a less defensive/conformant underlying implementation shared with the
> > public
> > API implementation where these kinds of overheads can be preserved.
> >
> > For the curious; the logs being processed for this boot are 48 * 8MiB on
> > SSD,
> > 1.8Ghz Core2 Duo, 4.12 kernel.
> >
> > Regards,
> > Vito Caputo
> > ___
> > systemd-devel mailing list
> > systemd-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/systemd-devel
> >
> -- 
> 
> Mantas Mikulėnas 
> Sent from my phone
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org

[systemd-devel] Significant performance loss caused by commit a65f06b: journal: return -ECHILD after a fork

[vcaputo]

In doing some casual journalctl profiling and stracing, it became apparent
that `journalctl -b --no-pager` runs across a significant quantity of logs,
~10% of the time was thrown away on getpid() calls due to commmit a65f06b.

As-is:
 # time ./journalctl -b --no-pager > /dev/null

 real0m11.033s
 user0m10.084s
 sys 0m0.943s


After changing journal_pid_changed() to simply return 1:
 # time ./journalctl -b --no-pager > /dev/null

  real0m9.641s
  user0m9.449s
  sys 0m0.191s


More system time is being expended in repeated getpid() calls than write(),
see the strace:

 12:51:56.939287 write(1, "Jul 07 09:25:23 x61s unknown"..., 57) = 57 <0.001276>
 12:51:56.940633 getpid()= 10713 <0.32>
 12:51:56.940732 getpid()= 10713 <0.12>
 12:51:56.940801 getpid()= 10713 <0.32>
 12:51:56.940867 getpid()= 10713 <0.41>
 12:51:56.940942 getpid()= 10713 <0.41>
 12:51:56.941047 getpid()= 10713 <0.12>
 12:51:56.941117 getpid()= 10713 <0.12>
 12:51:56.941185 getpid()= 10713 <0.11>
 12:51:56.941253 getpid()= 10713 <0.11>
 12:51:56.941320 getpid()= 10713 <0.39>
 12:51:56.941395 getpid()= 10713 <0.41>
 12:51:56.941494 getpid()= 10713 <0.11>
 12:51:56.941561 getpid()= 10713 <0.12>
 12:51:56.941629 getpid()= 10713 <0.39>
 12:51:56.942942 write(1, "Jul 07 09:25:23 x61s unknown"..., 57) = 57 <0.58>
 12:51:56.943052 getpid()= 10713 <0.39>
 12:51:56.943156 getpid()= 10713 <0.17>
 12:51:56.943230 getpid()= 10713 <0.18>
 12:51:56.943305 getpid()= 10713 <0.12>
 12:51:56.943374 getpid()= 10713 <0.17>
 12:51:56.943449 getpid()= 10713 <0.11>
 12:51:56.943517 getpid()= 10713 <0.11>
 12:51:56.943585 getpid()= 10713 <0.11>
 12:51:56.943652 getpid()= 10713 <0.11>
 12:51:56.943721 getpid()= 10713 <0.30>
 12:51:56.943796 getpid()= 10713 <0.41>
 12:51:56.943870 getpid()= 10713 <0.41>
 12:51:56.943944 getpid()= 10713 <0.41>
 12:51:56.944061 getpid()= 10713 <0.001334>
 12:51:56.945459 write(1, "Jul 07 09:25:23 x61s unknown"..., 56) = 56 <0.18>
 12:51:56.945544 getpid()= 10713 <0.17>
 12:51:56.945620 getpid()= 10713 <0.17>
 12:51:56.945694 getpid()= 10713 <0.12>
 12:51:56.945763 getpid()= 10713 <0.11>
 12:51:56.945832 getpid()= 10713 <0.12>
 12:51:56.945901 getpid()= 10713 <0.11>
 12:51:56.945969 getpid()= 10713 <0.11>
 12:51:56.946048 getpid()= 10713 <0.13>
 12:51:56.946118 getpid()= 10713 <0.24>
 12:51:56.946188 getpid()= 10713 <0.47>
 12:51:56.946277 getpid()= 10713 <0.41>
 12:51:56.946353 getpid()= 10713 <0.41>
 12:51:56.946428 getpid()= 10713 <0.40>
 12:51:56.946539 getpid()= 10713 <0.001363>

As this is public sd-journal API, it's somewhat set in stone.  However,
there's nothing preventing the systemd-internal tooling from linking with
a less defensive/conformant underlying implementation shared with the public
API implementation where these kinds of overheads can be preserved.

For the curious; the logs being processed for this boot are 48 * 8MiB on SSD,
1.8Ghz Core2 Duo, 4.12 kernel.

Regards,
Vito Caputo
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Github systemd issue 6237

[Kai Krakow]

Am Tue, 4 Jul 2017 21:23:01 + (UTC)
schrieb Alexander Bisogiannis :

> On Tue, 04 Jul 2017 17:21:01 +, Zbigniew Jędrzejewski-Szmek wrote:
> 
> > If you need root permissions to create a unit, then it's not a
> > security issue. An annoyance at most.  
> 
> The fact that you need to be root to create a unit file is irrelevant.
> 
> Systemd is running a service as a different user to what is defined
> in the unit file. 
> This is a bug and a local security issue, especially because it will
> run said service as root.
> 
> It might not warrant a CVE, although in my line of work this is 
> considered a security issue, but it is a bug and needs fixing.
> 
> The fix is to refuse to run the service, period.

There's nothing to fix because it already works that way: If you give
it a valid user name that does not exists, the system refuses to start
the unit with "user not found".

If you give it an invalid user name (leading digits, disallowed
characters), then it complains with a warning and continues to run as
if you specified no user (thus it runs as root).

The bug here is that a leading number will "convert" to the number and
it actually runs with the UID specified that way: 0day = 0, 7days = 7.
But this is not really a security concern as only root can create units
that contain a user - except you open exploits for that: But then you
have other problems then that.

Conclusion: Not a security issue. If you trick an admin into accepting
unit files without validating the contents, you are having other issues
than an issue with systemd.


> Is there any other place I can go to open a bug, or do I need to go
> to the upstream "vendor" bugzila?

Maybe open a new issue and suggest that the current "conversion" should
be upgraded from a warning to a fatal error. Give examples of behavior
you get and behavior you expect. Also give counter examples of behavior
that works as you expect. Don't try to troll, after all it's the
developers forum and it only works if people stay with the facts.
Otherwise it becomes unusable, nobody wants that.

Best way to get it into one of the next releases is to prepare a pull
request that fixes the issue.


-- 
Regards,
Kai

Replies to list-only preferred.


___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Timeout for 'Activating (start)' status

[Mikhail Kasimov]

Hello, everyone!


06.07.2017 19:17, Mikhail Kasimov пишет:
>
> 06.07.2017 19:12, Mikhail Kasimov пишет:
>> 06.07.2017 18:27, Michael Chapman пишет:
>>> On Fri, 7 Jul 2017, Mikhail Kasimov wrote:
 06.07.2017 17:18, Michael Chapman пишет:
> On Thu, 6 Jul 2017, Mikhail Kasimov wrote:
>> Hello!
>>
>> 've got an interesting trouble on timer-activated service --
>> 'systemctl
>> status' returns a log with 'Activating (start)' status:
>>
>> [1]
>> ==
>> k_mikhail@linux-mk500:~> systemctl status vba32update.service
>> ● vba32update.service - VBA32 Anti-Virus Update Service
>>   Loaded: loaded (/etc/systemd/system/vba32update.service; disabled;
>> vendor preset: disabled)
>>   Active: activating (start) since Чтв 2017-07-06 15:32:35 EEST;
>> 35min ago
>> Main PID: 6214 (vbaupdx)
>>Tasks: 1 (limit: 512)
>>   CGroup: /system.slice/vba32update.service
>>   └─6214 ./vbaupdx http://anti-virus.by/beta/update
>>
>> Июл 06 15:32:35 linux-mk500 systemd[1]: Starting VBA32 Anti-Virus
>> Update
>> Service...
>> Июл 06 15:32:35 linux-mk500 vbacl[6214]: Vba32 console scanner update
>> process started
>> Июл 06 15:32:35 linux-mk500 vbacl[6214]: Reading configuration options
>> from ./vbacl.ini
>> Июл 06 15:32:35 linux-mk500 vbacl[6214]: Using direct connection for
>> update
>> k_mikhail@linux-mk500:~>
>> ==
>>
>>
>> Simultaneously:
>> [2]
>> ==
>> k_mikhail@linux-mk500:~> systemctl list-units -t timer
>> UNIT LOAD   ACTIVE SUB DESCRIPTION
>> vba32update.timerloaded active running Runs VBA32 Update
>> Hourly
>> ==
>>
>> And that is normal. But 35 minutes for activating service is too long,
>> as for me.
> Is this a Type=oneshot service?
>
> It's a bit weird, but Type=oneshot services are not considered to be
> fully started until the ExecStart= command has exited. Until that time
> they're still "activating".
>
> You may be better off making the service Type=simple instead. A
> Type=simple service is fully started as soon as it has spawned the
> ExecStart= command.
 Yes, Type=oneshot, because, if Type=simple, service will be active
 permanently in case of its start. But this is not what is needed here --
 service should be started, then to check the updates of anti-virus
 software and then be successfully closed. Nothing more. Only one thing
 can impact here -- remote anti-virus server connection timeout. And if
 server connection timeout is set in wrong way, having a native systemd
 forced timeout for oneshot-services may be useful.
>>> I still think you want Type=simple there. A Type=simple service can
>>> exit when it's finished doing whatever it needs to do. The service
>>> will then transition back to its inactive state.
>>>
>>> Really, I think the only time Type=oneshot should be used is when you
>>> have a _sequence_ of units, where one unit shouldn't be started until
>>> the previous one has finished. You don't have that here.
>> Ok, here is Type=oneshot. And we can see two sessions of our service
>> (at1 7:06:52 and 18:06:57):
>> ==
>> [1]
>> Июл 06 17:06:52 linux-mk500 systemd[1]: Starting VBA32 Anti-Virus Update
>> Service...
>> Июл 06 17:06:52 linux-mk500 vbacl[8058]: Vba32 console scanner update
>> process started
>> Июл 06 17:06:52 linux-mk500 vbacl[8058]: Reading configuration options
>> from ./vbacl.ini
>> Июл 06 17:06:52 linux-mk500 vbacl[8058]: Using direct connection for update
>> Июл 06 17:06:56 linux-mk500 vbacl[8058]: Current dir is ./
>> Июл 06 17:06:56 linux-mk500 vbacl[8058]: Start update from
>> http://anti-virus.by/beta/update
>> Июл 06 17:06:56 linux-mk500 vbacl[8058]: Receiving file list
>> Июл 06 17:06:56 linux-mk500 vbacl[8058]: File list received
>> Июл 06 17:06:56 linux-mk500 vbacl[8058]: Update is not needed
>> Июл 06 17:06:57 linux-mk500 systemd[1]: Started VBA32 Anti-Virus Update
>> Service.
>>
>> [2]
>> Июл 06 18:06:57 linux-mk500 systemd[1]: Starting VBA32 Anti-Virus Update
>> Service...
>> Июл 06 18:06:57 linux-mk500 vbacl[8842]: Vba32 console scanner update
>> process started
>> Июл 06 18:06:57 linux-mk500 vbacl[8842]: Reading configuration options
>> from ./vbacl.ini
>> Июл 06 18:06:57 linux-mk500 vbacl[8842]: Using direct connection for update
>> Июл 06 18:07:00 linux-mk500 vbacl[8842]: Current dir is ./
>> Июл 06 18:07:00 linux-mk500 vbacl[8842]: Start update from
>> http://anti-virus.by/beta/update
>> Июл 06 18:07:00 linux-mk500 vbacl[8842]: Receiving file list
>> Июл 06 18:07:00 linux-mk500 vbacl[8842]: File list received
>> Июл 06 18:07:00 linux-mk500 vbacl[8842]: Update is not needed
>> Июл 06 18:07:00 linux-mk500 systemd[1]: Started VBA32 Anti-Virus Update
>> Service.
>> ==
>>
>>
>> And here when Type=simple:
>>