Re: [PATCH 1/1] Address regression in inet6_validate_link_af

From: Jonas Bonn
Date: Fri Jun 14 2019 - 04:25:21 EST




On 13/06/2019 16:13, Maxim Mikityanskiy wrote:
On 2019-06-13 09:45, Jonas Bonn wrote:
Hi Max,

On 12/06/2019 12:42, Maxim Mikityanskiy wrote:
On 2019-06-11 13:03, Jonas Bonn wrote:
Patch 7dc2bccab0ee37ac28096b8fcdc390a679a15841 introduces a regression
with systemd 241. In that revision, systemd-networkd fails to pass the
required flags early enough. This appears to be addressed in later
versions of systemd, but for users of version 241 where systemd-networkd
nonetheless worked with earlier kernels, the strict check introduced by
the patch causes a regression in behaviour.

This patch converts the failure to supply the required flags from an
error into a warning.
The purpose of my patch was to prevent a partial configuration update on
invalid input. -EINVAL was returned both before and after my patch, the
difference is that before my patch there was a partial update and a
warning.

Your patch basically makes mine pointless, because you revert the fix,
and now we'll have the same partial update and two warnings.

Unfortunately, yes...

So what you propose is a degradation.

Yes. You're not going to get an argument out of me here... :)



One more thing is that after applying your patch on top of mine, the
kernel won't return -EINVAL anymore on invalid input. Returning -EINVAL
is what happened before my patch, and also after my patch.

Yes, you're right, it would probably be better revert the entire patch
because the checks in set_link_af have been dropped on the assumption
that validate_link_af catches the badness.

We shouldn't introduce workarounds in the kernel for some temporary bugs
in old userspace. A regression was introduced in systemd: it started
sending invalid messages, didn't check the return code properly and
relied on an undefined behavior. It was then fixed in systemd. If the
kernel had all kinds of workarounds for all buggy software ever existed,
it would be a complete mess. The software used the API in a wrong way,
the expected behavior is failure, so it shouldn't expect anything else.
For me, the trade-off between fixing the kernel behavior and supporting
some old buggy software while keeping a UB in the kernel forever has an
obvious resolution.

You're missing the point: this is a regression in kernel behaviour. systemd v241 is a tagged release, included in downloadable releases of at least Yocto 2.7 and Fedora 30. Irregardless of whether systemd's implementation is buggy or not, it's functionality depends on the (undefined but deterministic) behaviour of a released kernel and therefore you can't just change that behaviour.

Like I said, I can get past this, so I'm happy to pretend that I didn't discover this and just patch my own system. Unfortunately, I suspect this isn't the last you'll hear about this.

/Jonas


Anyway, for the record, the error is:

systemd:Â Could not bring up interface... (invalid parameter)

And the solution is:Â Linux >= 5.2 requires systemd != v241.

If nobody else notices, that's good enough for me.


Please correct me if anything I say is wrong.

Nothing wrong, but it's still a regression.

/Jonas


Thanks,
Max