[Devel] [PATCH] autofs: fix autofs_v5_packet structure for compat mode

[Stanislav Kinsburskiy]

31.08.2017 15:05, Dmitry V. Levin пишет:
> On Thu, Aug 31, 2017 at 02:40:23PM +0300, Dmitry V. Levin wrote:
>> On Thu, Aug 31, 2017 at 01:48:27PM +0300, Stanislav Kinsburskiy wrote:
>>>
>>>
>>> 31.08.2017 13:38, Dmitry V. Levin пишет:
>>>> On Thu, Aug 31, 2017 at 02:11:34PM +0400, Stanislav Kinsburskiy wrote:
>>>>> Due to integer variables alignment size of struct autofs_v5_packet in 300
>>>>> bytes in 32-bit architectures (instead of 304 bytes in 64-bits architectures).
>>>>>
>>>>> This may lead to memory corruption (64 bits kernel always send 304 bytes,
>>>>> while 32-bit userspace application expects for 300).
>>>>>
>>>>> https://jira.sw.ru/browse/PSBM-71078
>>>>>
>>>>> Signed-off-by: Stanislav Kinsburskiy <skinsbursky at virtuozzo.com>
>>>>> ---
>>>>>  include/uapi/linux/auto_fs4.h |    2 ++
>>>>>  1 file changed, 2 insertions(+)
>>>>>
>>>>> diff --git a/include/uapi/linux/auto_fs4.h b/include/uapi/linux/auto_fs4.h
>>>>> index e02982f..8729a47 100644
>>>>> --- a/include/uapi/linux/auto_fs4.h
>>>>> +++ b/include/uapi/linux/auto_fs4.h
>>>>> @@ -137,6 +137,8 @@ struct autofs_v5_packet {
>>>>>  	__u32 pid;
>>>>>  	__u32 tgid;
>>>>>  	__u32 len;
>>>>> +	__u32 blob;		/* This is needed to align structure up to 8
>>>>> +				   bytes for ALL archs including 32-bit */
>>>>>  	char name[NAME_MAX+1];
>>>>>  };
>>>>
>>>> This change breaks ABI because it changes offsetof(struct autofs_v5_packet, name).
>>>> If you need to fix the alignment, use  __attribute__((aligned(8))).
>>>>
>>>
>>> Nice to know you're watching.
>>> Yes, attribute is better.
>>> But how ABI is broken? On x86_64 this alignment is implied, so nothing is changed.
>>
>> Your change increases offsetof(struct autofs_v5_packet, name) by 4 on all
>> architectures.  On architectures where the structure is 32-bit aligned
>> this also leads to increase of its size by 4.
>>
>>>> An alignment change would also be an ABI breakage on 32-bit architectures,
>>>> though.
>>>>
>>>
>>> True.
>>> But from my POW better have it working on 64bit archs for 32bit apps.
>>> But anyway, upstream guys will device, whether they want 32-bit autofs applications properly work on 64 or 32 bits.
>>
>> Let's fix old bugs without introducing new bugs.
>> The right fix here seems to be a compat structure, that is, both 64-bit
>> and 32-bit kernels should send the same 32-bit aligned structure, and
>> it has to be the same structure sent by traditional 32-bit kernels.
> 
> Alternatively, a much more simple fix would be to change 64-bit kernels
> not to send the trailing 4 padding bytes of 64-bit aligned
> struct autofs_v5_packet.  That is, just send
> offsetofend(struct autofs_v5_packet, name) bytes instead of
> sizeof(struct autofs_v5_packet) regardless of architecture.
> 

Fair enough, thanks!
But this approach won't work, because autofs pipe has O_DIRECT flag.
Compat structure looks more promising, but not yet clear to me, how to define one properly.
Probably by replacing "len" with char array like this:

/* autofs v5 common packet struct */
struct autofs_v5_packet {
        struct autofs_packet_hdr hdr;
        autofs_wqt_t wait_queue_token;
        __u32 dev;
        __u64 ino;
        __u32 uid;
        __u32 gid;
        __u32 pid;
        __u32 tgid;
        __u8  len[4];
        char name[NAME_MAX+1];
};

What do you think?