[Devel] [RFC PATCH 2/2] autofs: sent 32-bit sized packet for 32-bit process

[Stanislav Kinsburskiy]

03.09.2017 21:23, Dmitry V. Levin P?P8QP5Q:
> On Sat, Sep 02, 2017 at 02:26:26PM +0300, Stanislav Kinsburskiy wrote:
>> 01.09.2017 21:23, Dmitry V. Levin P?P8QP5Q:
>>> On Fri, Sep 01, 2017 at 05:02:45PM +0300, Stanislav Kinsburskiy wrote:
>>>> 01.09.2017 16:53, Dmitry V. Levin P?P8QP5Q:
>>>>> On Fri, Sep 01, 2017 at 12:15:17PM +0300, Stanislav Kinsburskiy wrote:
>>>>>> 31.08.2017 20:22, Dmitry V. Levin P?P8QP5Q:
>>>>>>> On Thu, Aug 31, 2017 at 05:57:11PM +0400, Stanislav Kinsburskiy wrote:
>>>>>>>> The structure autofs_v5_packet (except name) is not aligned by 8 bytes, which
>>>>>>>> lead to different sizes in 32 and 64-bit architectures.
>>>>>>>> Let's form 32-bit compatible packet when daemon has 32-bit addressation.
>>>>>>>>
>>>>>>>> Signed-off-by: Stanislav Kinsburskiy <skinsbursky at virtuozzo.com>
>>>>>>>> ---
>>>>>>>>  fs/autofs4/waitq.c |   11 +++++++++--
>>>>>>>>  1 file changed, 9 insertions(+), 2 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/fs/autofs4/waitq.c b/fs/autofs4/waitq.c
>>>>>>>> index 309ca6b..484cf2e 100644
>>>>>>>> --- a/fs/autofs4/waitq.c
>>>>>>>> +++ b/fs/autofs4/waitq.c
>>>>>>>> @@ -153,12 +153,19 @@ static void autofs4_notify_daemon(struct autofs_sb_info *sbi,
>>>>>>>>  	{
>>>>>>>>  		struct autofs_v5_packet *packet = &pkt.v5_pkt.v5_packet;
>>>>>>>>  		struct user_namespace *user_ns = sbi->pipe->f_cred->user_ns;
>>>>>>>> +		size_t name_offset;
>>>>>>>>  
>>>>>>>> -		pktsz = sizeof(*packet);
>>>>>>>> +		if (sbi->is32bit)
>>>>>>>> +			name_offset = offsetof(struct autofs_v5_packet, len) +
>>>>>>>> +				      sizeof(packet->len);
>>>>>>>> +		else
>>>>>>>> +			name_offset = offsetof(struct autofs_v5_packet, name);
>>>>>>>
>>>>>>> This doesn't help at all because the offset of struct autofs_v5_packet.name
>>>>>>> does not change.
>>>>>>>
>>>>>>>> +		pktsz = name_offset + sizeof(packet->name);
>>>>>>>
>>>>>>> What changes is pktsz: it's either sizeof(struct autofs_v5_packet)
>>>>>>> or 4 bytes less, depending on the architecture.
>>>>>>
>>>>>> Indeed. Thanks!
>>>>>>
>>>>>>> For example,
>>>>>>>
>>>>>>> #ifdef CONFIG_COMPAT
>>>>>>> 	if (__alignof__(compat_u64) < __alignof__(u64) && sbi->is32bit)
>>>>>>
>>>>>> Unfortunately I don't get this "alignof" checks.
>>>>>> The intention of "is32bit" was to define this compat case completely.
>>>>>> Why are they required?
>>>>>
>>>>> On some 32-bit architectures like arm, u64 is 64-bit aligned, on others
>>>>> like x86 it is not.  This alignof check ensures that compat 32-bit
>>>>> architectures with 64-bit alignment are not going to be broken
>>>>> by the change.
>>>>
>>>> Thanks for the explanation!
>>>> But looks like the issue is hidden so deep (thanks to O_DIRECT), that becomes unimportant.
>>>
>>> I'm not quite sure how O_DIRECT makes this unimportant.
>>> For example, automount_dispatch_io from systemd tries to read exactly
>>> sizeof(union autofs_v5_packet_union) bytes and fails in case of short read.
>>
>> That's weird. Can you reproduce it?
>> The problem is that kernel always send arch packet (i.e. 300 bytes for x86, or 304 for x86_64).
> 
> Well, if sending a lengthy packet is not a problem, what is the problem
> you were trying to solve by patching struct autofs_v5_packet?
> 

Well, nothing anymore. I took some time to realize, that this issue doesn't hurt anyone.
So I fixed our test instead.