Re: [Devel] [PATCH] autofs: fix autofs_v5_packet structure for compat mode
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
> ---
> 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?
___
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
Re: [Devel] [PATCH] autofs: fix autofs_v5_packet structure for compat mode
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
> > >> ---
> > >> 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.
--
ldv
signature.asc
Description: PGP signature
___
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
Re: [Devel] [PATCH] autofs: fix autofs_v5_packet structure for compat mode
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
> >> ---
> >> 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.
--
ldv
signature.asc
Description: PGP signature
___
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
Re: [Devel] [PATCH] autofs: fix autofs_v5_packet structure for compat mode
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
>> ---
>> 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.
> 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.
___
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
Re: [Devel] [PATCH] autofs: fix autofs_v5_packet structure for compat mode
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
> ---
> 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))).
An alignment change would also be an ABI breakage on 32-bit architectures,
though.
--
ldv
signature.asc
Description: PGP signature
___
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
Re: [Devel] [PATCH] autofs: fix autofs_v5_packet structure for compat mode
Yes
31.08.2017 13:37, Konstantin Khorenko пишет:
> Will you send it to mainstream as well?
>
> --
> Best regards,
>
> Konstantin Khorenko,
> Virtuozzo Linux Kernel Team
>
> On 08/31/2017 01:11 PM, 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
>> ---
>> 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];
>> };
>>
>>
>> .
>>
___
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
Re: [Devel] [PATCH] autofs: fix autofs_v5_packet structure for compat mode
Will you send it to mainstream as well?
--
Best regards,
Konstantin Khorenko,
Virtuozzo Linux Kernel Team
On 08/31/2017 01:11 PM, 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
---
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];
};
.
___
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
[Devel] [PATCH] autofs: fix autofs_v5_packet structure for compat mode
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
---
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];
};
___
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
