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 <skinsbur...@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.


-- 
ldv

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Reply via email to