On Fri, 2021-01-15 at 11:59 -0500, Bruce Ashfield wrote:
> On Fri, Jan 15, 2021 at 11:31 AM Luca Boccassi
> <[email protected]> wrote:
> > On Fri, 2021-01-15 at 11:07 -0500, Bruce Ashfield wrote:
> > > On Fri, Jan 15, 2021 at 10:37 AM Luca Boccassi via
> > > lists.openembedded.org
> > > <[email protected]> wrote:
> > > > On Fri, 2021-01-15 at 09:47 -0500, Paul Gortmaker wrote:
> > > > > [Re: [PATCH] systemd: dont spew hidepid mount errors for kernels < 
> > > > > v5.8] On 15/01/2021 (Fri 09:57) Luca Boccassi wrote:
> > > > > 
> > > > > > On Fri, 2021-01-15 at 08:37 +0000, Richard Purdie wrote:
> > > > > > > On Fri, 2021-01-15 at 00:26 -0500, Paul Gortmaker wrote:
> > > > > > > > Recent systemd started using ascii args to "hidepid=" mount 
> > > > > > > > options
> > > > > > > > for proc fs - unconditionally -- even though kernels older than 
> > > > > > > > v5.8
> > > > > > > > emit an error message on each attempt:
> > > > > > > > 
> > > > > > > > root@qemux86-64:~# cat /proc/version
> > > > > > > > Linux version 5.4.87-yocto-standard (oe-user@oe-host) (gcc 
> > > > > > > > version 10.2.0 (GCC)) #1 SMP PREEMPT Fri Jan 8 01:47:13 UTC 2021
> > > > > > > > root@qemux86-64:~# dmesg|grep proc:
> > > > > > > > [   29.487995] proc: Bad value for 'hidepid'
> > > > > > > > [   43.170571] proc: Bad value for 'hidepid'
> > > > > > > > [   44.175615] proc: Bad value for 'hidepid'
> > > > > > > > [   46.213300] proc: Bad value for 'hidepid'
> > > > > > > > root@qemux86-64:~#
> > > > > > 
> > > > > > Wouldn't it be better to patch the kernel to downgrade that message 
> > > > > > to
> > > > > > debug level?
> > > > > 
> > > > > The problem is that the breakage is forced upon older kernels, so 
> > > > > you'd
> > > > > have to effectively mainline that kind of "fix" to v5.12 (where there 
> > > > > is
> > > > > no problem) and then you could in theory request it for v5.4 
> > > > > linux-stable
> > > > > according to "stable rules".
> > > > 
> > > > The patch could be downstream for older kernels, just like this one is.
> > > > With the difference that it would be temporary.
> > > 
> > > But the coverage is impossible, since there are so many
> > > different kernel trees. So a kernel solution is a non-starter.
> > 
> > IMHO a long term solution can only go through the kernel. There will be
> > more instances like this in the future, given the fundamental issue is
> > the lack of a clear EOPNOTSUPP-like interface. I realise the mount
> > interface is ancient at this point, and difficult to change. We are
> > keeping an eye on the evolution of the new interface, but it's not
> > there yet (eg: no read-only bind mounts).
> 
> I don't disagree .. but I'm only commenting on this one older kernel
> issue, so there's nothing to see in the new kernels where I spend
> 99% of my time. The interfaces in the kernel will absolutely evolve
> in the newer kernels, and that's a good thing.
> 
> > > > > Besides, if something with root/mount priv. is consistently trying to
> > > > > drive a square peg into a round hole - by trying to mount something 
> > > > > and
> > > > > failing -- it should be something that a sysadmin would want to
> > > > > investigate.  Which is precisely why I am here now.  I think burying 
> > > > > it
> > > > > at debug level is counterproductive to that.
> > > > 
> > > > Well the real issue is that there's no way to get a clean "we don't
> > > > support this option" out of certain kernel APIs, so one has to guess
> > > > and see what happens. Sometimes it's even worse, and flags like
> > > > NOSYMFOLLOW get silently ignored if they are not supported, which is
> > > > not great for security-related settings...
> > > > 
> > > > Anyway, these warnings only appear if ProtectProc and/or ProcSubset are
> > > > set to something other than the default value. Why not simply add a
> > > > top-level drop-in in /etc that forces them to be disabled in all
> > > > services if you have an older kernel? Something like this:
> > > > 
> > > > /etc/systemd/system/service.d/10-override-protect-proc.conf
> > > > [Service]
> > > > ProtectProc=default
> > > > ProcSubset=all
> > > > 
> > > > And then you can drop it when you upgrade your kernel. Isn't this a
> > > > better option than taking on permanent technical debt?
> > > 
> > > That's even more fiddly than Paul's patch. It relies on much more
> > > of someone's distro configuration.
> > 
> > But what is the combination of a kernel version + systemd version if
> > not someone's distro configuration?
> > I mean, it could even be added dynamically by the poky recipe at build
> > time, given the kernel version is known at that point.
> 
> The kernel isn't currently a dependency of systemd and you'd be going
> outside of the recipe namespace (but maybe I'm missing something, I
> spent about a minute refreshing my memory on the recipes) .
> Something at the image/rootfs level, gets brittle and doesn't scale
> (but again, maybe I'm overlooking some technique).

There's the get_kernelversion* functions available to recipes,
reasonably widely used already - so something that checks it and
conditionally installs the drop-in should work fine, in theory?

> > Surely a three lines piece of configuration is better to handle than a
> > downstream patch?
> > 
> 
> Not in my opinion. I have no interest in extra moving parts and having
> something injected into my userspace/init config.

There's half a million things "injected in your userspace/init
config"... I mean, that's what a distro does? This is exactly why this
config knobs were added, so that distro builders can customize the
behaviour to their liking.

> > > But it is true that you can throw it away eventually, assuming
> > > someone actually knows that it is there.
> > > 
> > > I wouldn't call this patch much of a technical debt, but if it starts
> > > gumming up systemd upgrades, it's easy to revisit.
> > 
> > It's in a security-critical area that we are constantly improving and
> > expanding.
> 
> That code block isn't particularly complex and it is self contained,
> so really, there's not much to see there. But I'm not in the business
> of predicting anything, I'm in the wait and see camp (AUH will be the
> first to pick it up if it causes patch problems, etc).
> 
> Richard can either take the change or not, I'm just supporting how
> it has currently been submitted as chances are that no further
> revisions are coming from Paul, so it can live or die as-is.

Even if a patch was preferred to a config, the current patch changes
too much and should really be reduced in scope, as commented in the
other email.

-- 
Kind regards,
Luca Boccassi

Attachment: signature.asc
Description: This is a digitally signed message part

-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#146846): 
https://lists.openembedded.org/g/openembedded-core/message/146846
Mute This Topic: https://lists.openembedded.org/mt/79695930/21656
Group Owner: [email protected]
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub 
[[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to