[systemd-devel] [PATCH] Circumvent autofs_v5_packet_union size bug

[Kay Sievers]

On Mon, Nov 14, 2011 at 02:38, Ian Kent <raven at themaw.net> wrote:
> On Fri, 2011-11-11 at 14:33 +0100, Kay Sievers wrote:
>> On Tue, Sep 20, 2011 at 14:26, Kay Sievers <kay.sievers at vrfy.org> wrote:
>> > On Tue, Sep 20, 2011 at 12:58, Lennart Poettering
>> > <lennart at poettering.net> wrote:
>> >> On Tue, 20.09.11 11:36, Ian Kent (raven at themaw.net) wrote:
>> >>
>> >>> > This is a bug in the kernel, and it should be fixed in the kernel (which
>> >>> > would mean the kernel folks have to define a new version of this
>> >>> > struct/ioctl which fixes this). If that's defined we can then add
>> >>> > support for this into systemd. Could you file a bug about this please on
>> >>> > the kernel bugzilla? (please cc me, or post the url here!)
>> >>>
>> >>> Yes, it is a mistake that I made but it is a bad idea to change
>> >>> something that will cause widespread failure of existing binaries and
>> >>> that's why the discrepancy persists and will continue to do so.
>> >>
>> >> I am not expecting this to be changed in the kernel. Instead I expect
>> >> that the kernel API is extended with a correct version of the same data
>> >> structure. If the kernel broke it the kernel should fix it.
>> >>
>> >>> I chose to compensate for it in user space and to advise anyone that
>> >>> encountered the problem on how to handle it and I have had no reports of
>> >>> problems in autofs since I added the size calculation (which was a long
>> >>> time ago now).
>> >>
>> >> Well, it's a workaround in userspace. Fix the kernel. Add a second union
>> >> or something which can be used by newer clients.
>> >>
>> >>> > I am sorry but I am pretty sure I want to keep the compat kludges for
>> >>> > broken things in systemd at a minimum, adding such a work around looks
>> >>> > like the wrong way to me. We generally try to fix problems where they
>> >>> > are these days, not where we notice them.
>> >>>
>> >>> Sure, it is unfortunate but, as far as the autofs kernel module is
>> >>> concerned the decision about this was made a long time ago and, yes it
>> >>> isn't what we want but the breakage it would have caused then and the
>> >>> breakage it would cause now is too great.
>> >>
>> >> Adding a corrected version will not break anything. This is not about
>> >> replacing the current borked interface, but simply by adding a fixed
>> >> version that we can use from userspace without ugly compat glue.
>> >>
>> >> Fix the problems where they are, don't work around them where they aren't.
>> >
>> > I can only second that.
>> >
>> > Please add a new definition, leave the old around, and do not require
>> > userspace to do wild things, like compare static lists of
>> > architectures to work around things that can be expected to just work.
>>
>> Ian, is this fixed in the meantime? If not, mind fixing the kernel bug
>> and introduce an non-broken version of the kernel interface.
>
> I spent some time having a look at how I might do it but I didn't end up
> with anything suitable.
>
> How exactly did you think I would be able to do it without introducing
> breakage?

We thought by introducing a non-broken version of the ioctl. Like
Thomas Meyer posted earlier in this thread. I did not look at the
patch details, but what's the problem with that approach?

>> We like to have this fixed in systemd, but are still reluctant to
>> compile lists of static 32 vs. 64 architecture bit matches in init.
>
> Why would you need to do that, it's easy to work out the expected packet
> size at program start with the function I posted.

This is about 'init', which is far more exposed than an autofs daemon.
We really don't want to compile static lists of architectures into
'init', to work around such kernel bugs. Linux has a new architecture
every month, and some of them switch from 32 to 64 bit. That fix
belongs into the kernel, not in paper-over fixes in all userspace
tools which use autofs.

> Even if I had a way to
> do it you would still need to use the function since you won't know what
> kernel version your client users will be using.

We have no problem to require a recent kernel and a minimal protocol
version of autofs to fix these issues. Is that what you mean?

Kay