The problem is that in compat mode struct autofs_v5_packet has to have different size
(i.e. 4 bytes less).
This is RFC because:
1) This issue is hidden, because autofs pipe has O_DIRECT and the rest of the
epacket is truncated when read.
2) X86 arch doesn't have is_compat_task() helper
3) It's now clear, what to do if "pgrp" option is specified.
The following series implements...
---
Stanislav Kinsburskiy (2):
autofs: set compat flag on sbi when daemon uses 32bit addressation
autofs: sent 32-bit sized packet for 32-bit process
fs/autofs4/autofs_i.h | 3 +++
fs/autofs4/dev-ioctl.c | 3 +++
fs/autofs4/inode.c | 4 +++-
fs/autofs4/waitq.c | 5 +++++
4 files changed, 14 insertions(+), 1 deletion(-)
Signed-off-by: Stanislav Kinsburskiy <[email protected]>
---
fs/autofs4/autofs_i.h | 3 +++
fs/autofs4/dev-ioctl.c | 3 +++
fs/autofs4/inode.c | 4 +++-
3 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/fs/autofs4/autofs_i.h b/fs/autofs4/autofs_i.h
index 4737615..3da105f 100644
--- a/fs/autofs4/autofs_i.h
+++ b/fs/autofs4/autofs_i.h
@@ -120,6 +120,9 @@ struct autofs_sb_info {
struct list_head active_list;
struct list_head expiring_list;
struct rcu_head rcu;
+#ifdef CONFIG_COMPAT
+ unsigned is32bit:1;
+#endif
};
static inline struct autofs_sb_info *autofs4_sbi(struct super_block *sb)
diff --git a/fs/autofs4/dev-ioctl.c b/fs/autofs4/dev-ioctl.c
index b7c816f..467d6c4 100644
--- a/fs/autofs4/dev-ioctl.c
+++ b/fs/autofs4/dev-ioctl.c
@@ -397,6 +397,9 @@ static int autofs_dev_ioctl_setpipefd(struct file *fp,
sbi->pipefd = pipefd;
sbi->pipe = pipe;
sbi->catatonic = 0;
+#ifdef CONFIG_COMPAT
+ sbi->is32bit = is_compat_task();
+#endif
}
out:
put_pid(new_pid);
diff --git a/fs/autofs4/inode.c b/fs/autofs4/inode.c
index 09e7d68..21d3c0b 100644
--- a/fs/autofs4/inode.c
+++ b/fs/autofs4/inode.c
@@ -301,7 +301,9 @@ int autofs4_fill_super(struct super_block *s, void *data, int silent)
} else {
sbi->oz_pgrp = get_task_pid(current, PIDTYPE_PGID);
}
-
+#ifdef CONFIG_COMPAT
+ sbi->is32bit = is_compat_task();
+#endif
if (autofs_type_trigger(sbi->type))
__managed_dentry_set_managed(root);
The structure autofs_v5_packet (except name) is not aligned by 8 bytes, which
leads to different sizes in 32 and 64-bit architectures.
Let's form 32-bit compatible packet when daemon has 32-bit addressation.
Suggested-by: Dmitry V. Levin <[email protected]>
Signed-off-by: Stanislav Kinsburskiy <[email protected]>
---
fs/autofs4/waitq.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/fs/autofs4/waitq.c b/fs/autofs4/waitq.c
index 24a58bf..1f9b7d8 100644
--- a/fs/autofs4/waitq.c
+++ b/fs/autofs4/waitq.c
@@ -151,6 +151,11 @@ 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;
+#ifdef CONFIG_COMPAT
+ if (sbi->is32bit)
+ pktsz = offsetofend(struct autofs_v5_packet, name);
+ else
+#endif
pktsz = sizeof(*packet);
packet->wait_queue_token = wq->wait_queue_token;
On 01/09/17 19:21, Stanislav Kinsburskiy wrote:
> The problem is that in compat mode struct autofs_v5_packet has to have different size
> (i.e. 4 bytes less).
I regret (several times over) my original decision to not make v5 packets
packed ....
I have to say the description of the problem is not all that good.
>
> This is RFC because:
> 1) This issue is hidden, because autofs pipe has O_DIRECT and the rest of the
> epacket is truncated when read.
> 2) X86 arch doesn't have is_compat_task() helper
> 3) It's now clear, what to do if "pgrp" option is specified.
I don't understand what the "pgrp" option has to do with this?
>
> The following series implements...
>
> ---
>
> Stanislav Kinsburskiy (2):
> autofs: set compat flag on sbi when daemon uses 32bit addressation
> autofs: sent 32-bit sized packet for 32-bit process
>
>
> fs/autofs4/autofs_i.h | 3 +++
> fs/autofs4/dev-ioctl.c | 3 +++
> fs/autofs4/inode.c | 4 +++-
> fs/autofs4/waitq.c | 5 +++++
> 4 files changed, 14 insertions(+), 1 deletion(-)
>
On 01/09/17 19:21, Stanislav Kinsburskiy wrote:
> Signed-off-by: Stanislav Kinsburskiy <[email protected]>
> ---
> fs/autofs4/autofs_i.h | 3 +++
> fs/autofs4/dev-ioctl.c | 3 +++
> fs/autofs4/inode.c | 4 +++-
> 3 files changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/fs/autofs4/autofs_i.h b/fs/autofs4/autofs_i.h
> index 4737615..3da105f 100644
> --- a/fs/autofs4/autofs_i.h
> +++ b/fs/autofs4/autofs_i.h
> @@ -120,6 +120,9 @@ struct autofs_sb_info {
> struct list_head active_list;
> struct list_head expiring_list;
> struct rcu_head rcu;
> +#ifdef CONFIG_COMPAT
> + unsigned is32bit:1;
> +#endif
> };
>
> static inline struct autofs_sb_info *autofs4_sbi(struct super_block *sb)
> diff --git a/fs/autofs4/dev-ioctl.c b/fs/autofs4/dev-ioctl.c
> index b7c816f..467d6c4 100644
> --- a/fs/autofs4/dev-ioctl.c
> +++ b/fs/autofs4/dev-ioctl.c
> @@ -397,6 +397,9 @@ static int autofs_dev_ioctl_setpipefd(struct file *fp,
> sbi->pipefd = pipefd;
> sbi->pipe = pipe;
> sbi->catatonic = 0;
> +#ifdef CONFIG_COMPAT
> + sbi->is32bit = is_compat_task();
> +#endif
> }
> out:
> put_pid(new_pid);
> diff --git a/fs/autofs4/inode.c b/fs/autofs4/inode.c
> index 09e7d68..21d3c0b 100644
> --- a/fs/autofs4/inode.c
> +++ b/fs/autofs4/inode.c
> @@ -301,7 +301,9 @@ int autofs4_fill_super(struct super_block *s, void *data, int silent)
> } else {
> sbi->oz_pgrp = get_task_pid(current, PIDTYPE_PGID);
> }
> -
> +#ifdef CONFIG_COMPAT
> + sbi->is32bit = is_compat_task();
> +#endif
> if (autofs_type_trigger(sbi->type))
> __managed_dentry_set_managed(root);
>
>
Not sure about this.
Don't you think it would be better to avoid the in code #ifdefs by doing some
checks and defines in the header file and defining what's need to just use
is_compat_task().
Not sure 2 patches are needed for this either ......
Ian
14.09.2017 02:38, Ian Kent пишет:
> On 01/09/17 19:21, Stanislav Kinsburskiy wrote:
>> Signed-off-by: Stanislav Kinsburskiy <[email protected]>
>> ---
>> fs/autofs4/autofs_i.h | 3 +++
>> fs/autofs4/dev-ioctl.c | 3 +++
>> fs/autofs4/inode.c | 4 +++-
>> 3 files changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/autofs4/autofs_i.h b/fs/autofs4/autofs_i.h
>> index 4737615..3da105f 100644
>> --- a/fs/autofs4/autofs_i.h
>> +++ b/fs/autofs4/autofs_i.h
>> @@ -120,6 +120,9 @@ struct autofs_sb_info {
>> struct list_head active_list;
>> struct list_head expiring_list;
>> struct rcu_head rcu;
>> +#ifdef CONFIG_COMPAT
>> + unsigned is32bit:1;
>> +#endif
>> };
>>
>> static inline struct autofs_sb_info *autofs4_sbi(struct super_block *sb)
>> diff --git a/fs/autofs4/dev-ioctl.c b/fs/autofs4/dev-ioctl.c
>> index b7c816f..467d6c4 100644
>> --- a/fs/autofs4/dev-ioctl.c
>> +++ b/fs/autofs4/dev-ioctl.c
>> @@ -397,6 +397,9 @@ static int autofs_dev_ioctl_setpipefd(struct file *fp,
>> sbi->pipefd = pipefd;
>> sbi->pipe = pipe;
>> sbi->catatonic = 0;
>> +#ifdef CONFIG_COMPAT
>> + sbi->is32bit = is_compat_task();
>> +#endif
>> }
>> out:
>> put_pid(new_pid);
>> diff --git a/fs/autofs4/inode.c b/fs/autofs4/inode.c
>> index 09e7d68..21d3c0b 100644
>> --- a/fs/autofs4/inode.c
>> +++ b/fs/autofs4/inode.c
>> @@ -301,7 +301,9 @@ int autofs4_fill_super(struct super_block *s, void *data, int silent)
>> } else {
>> sbi->oz_pgrp = get_task_pid(current, PIDTYPE_PGID);
>> }
>> -
>> +#ifdef CONFIG_COMPAT
>> + sbi->is32bit = is_compat_task();
>> +#endif
>> if (autofs_type_trigger(sbi->type))
>> __managed_dentry_set_managed(root);
>>
>>
>
> Not sure about this.
>
> Don't you think it would be better to avoid the in code #ifdefs by doing some
> checks and defines in the header file and defining what's need to just use
> is_compat_task().
>
Yes, might be...
> Not sure 2 patches are needed for this either ......
>
Well, I found this issue occasionally.
And, frankly speaking, it's not clear to me, whether this issue is important at all, so I wanted to clarify this first.
Thanks to O_DIRECT, the only way to catch the issue is to try to read more, than expected, in compat task (that's how I found it).
I don't see any other flaw so far. And if so, that, probably, we shouldn't care about the issue at all.
What do you think?
> Ian
>
On 14/09/17 17:24, Stanislav Kinsburskiy wrote:
>
>
> 14.09.2017 02:38, Ian Kent пишет:
>> On 01/09/17 19:21, Stanislav Kinsburskiy wrote:
>>> Signed-off-by: Stanislav Kinsburskiy <[email protected]>
>>> ---
>>> fs/autofs4/autofs_i.h | 3 +++
>>> fs/autofs4/dev-ioctl.c | 3 +++
>>> fs/autofs4/inode.c | 4 +++-
>>> 3 files changed, 9 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/fs/autofs4/autofs_i.h b/fs/autofs4/autofs_i.h
>>> index 4737615..3da105f 100644
>>> --- a/fs/autofs4/autofs_i.h
>>> +++ b/fs/autofs4/autofs_i.h
>>> @@ -120,6 +120,9 @@ struct autofs_sb_info {
>>> struct list_head active_list;
>>> struct list_head expiring_list;
>>> struct rcu_head rcu;
>>> +#ifdef CONFIG_COMPAT
>>> + unsigned is32bit:1;
>>> +#endif
>>> };
>>>
>>> static inline struct autofs_sb_info *autofs4_sbi(struct super_block *sb)
>>> diff --git a/fs/autofs4/dev-ioctl.c b/fs/autofs4/dev-ioctl.c
>>> index b7c816f..467d6c4 100644
>>> --- a/fs/autofs4/dev-ioctl.c
>>> +++ b/fs/autofs4/dev-ioctl.c
>>> @@ -397,6 +397,9 @@ static int autofs_dev_ioctl_setpipefd(struct file *fp,
>>> sbi->pipefd = pipefd;
>>> sbi->pipe = pipe;
>>> sbi->catatonic = 0;
>>> +#ifdef CONFIG_COMPAT
>>> + sbi->is32bit = is_compat_task();
>>> +#endif
>>> }
>>> out:
>>> put_pid(new_pid);
>>> diff --git a/fs/autofs4/inode.c b/fs/autofs4/inode.c
>>> index 09e7d68..21d3c0b 100644
>>> --- a/fs/autofs4/inode.c
>>> +++ b/fs/autofs4/inode.c
>>> @@ -301,7 +301,9 @@ int autofs4_fill_super(struct super_block *s, void *data, int silent)
>>> } else {
>>> sbi->oz_pgrp = get_task_pid(current, PIDTYPE_PGID);
>>> }
>>> -
>>> +#ifdef CONFIG_COMPAT
>>> + sbi->is32bit = is_compat_task();
>>> +#endif
>>> if (autofs_type_trigger(sbi->type))
>>> __managed_dentry_set_managed(root);
>>>
>>>
>>
>> Not sure about this.
>>
>> Don't you think it would be better to avoid the in code #ifdefs by doing some
>> checks and defines in the header file and defining what's need to just use
>> is_compat_task().
>>
>
> Yes, might be...
>
>> Not sure 2 patches are needed for this either ......
>>
>
> Well, I found this issue occasionally.
I'm wondering what the symptoms are?
> And, frankly speaking, it's not clear to me, whether this issue is important at all, so I wanted to clarify this first.
> Thanks to O_DIRECT, the only way to catch the issue is to try to read more, than expected, in compat task (that's how I found it).
Right, the O_DIRECT patch from Linus was expected to fix the structure
alignment problem. The stuct field offsets are ok aren't they?
> I don't see any other flaw so far. And if so, that, probably, we shouldn't care about the issue at all.
> What do you think?
If we are seeing hangs, incorrect struct fields or similar something
should be done about it but if all is actually working ok then the
O_DIRECT fix is doing it's job and further changes aren't necessary.
Ian
14.09.2017 13:29, Ian Kent пишет:
> On 14/09/17 17:24, Stanislav Kinsburskiy wrote:
>>
>>
>> 14.09.2017 02:38, Ian Kent пишет:
>>> On 01/09/17 19:21, Stanislav Kinsburskiy wrote:
>>>> Signed-off-by: Stanislav Kinsburskiy <[email protected]>
>>>> ---
>>>> fs/autofs4/autofs_i.h | 3 +++
>>>> fs/autofs4/dev-ioctl.c | 3 +++
>>>> fs/autofs4/inode.c | 4 +++-
>>>> 3 files changed, 9 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/fs/autofs4/autofs_i.h b/fs/autofs4/autofs_i.h
>>>> index 4737615..3da105f 100644
>>>> --- a/fs/autofs4/autofs_i.h
>>>> +++ b/fs/autofs4/autofs_i.h
>>>> @@ -120,6 +120,9 @@ struct autofs_sb_info {
>>>> struct list_head active_list;
>>>> struct list_head expiring_list;
>>>> struct rcu_head rcu;
>>>> +#ifdef CONFIG_COMPAT
>>>> + unsigned is32bit:1;
>>>> +#endif
>>>> };
>>>>
>>>> static inline struct autofs_sb_info *autofs4_sbi(struct super_block *sb)
>>>> diff --git a/fs/autofs4/dev-ioctl.c b/fs/autofs4/dev-ioctl.c
>>>> index b7c816f..467d6c4 100644
>>>> --- a/fs/autofs4/dev-ioctl.c
>>>> +++ b/fs/autofs4/dev-ioctl.c
>>>> @@ -397,6 +397,9 @@ static int autofs_dev_ioctl_setpipefd(struct file *fp,
>>>> sbi->pipefd = pipefd;
>>>> sbi->pipe = pipe;
>>>> sbi->catatonic = 0;
>>>> +#ifdef CONFIG_COMPAT
>>>> + sbi->is32bit = is_compat_task();
>>>> +#endif
>>>> }
>>>> out:
>>>> put_pid(new_pid);
>>>> diff --git a/fs/autofs4/inode.c b/fs/autofs4/inode.c
>>>> index 09e7d68..21d3c0b 100644
>>>> --- a/fs/autofs4/inode.c
>>>> +++ b/fs/autofs4/inode.c
>>>> @@ -301,7 +301,9 @@ int autofs4_fill_super(struct super_block *s, void *data, int silent)
>>>> } else {
>>>> sbi->oz_pgrp = get_task_pid(current, PIDTYPE_PGID);
>>>> }
>>>> -
>>>> +#ifdef CONFIG_COMPAT
>>>> + sbi->is32bit = is_compat_task();
>>>> +#endif
>>>> if (autofs_type_trigger(sbi->type))
>>>> __managed_dentry_set_managed(root);
>>>>
>>>>
>>>
>>> Not sure about this.
>>>
>>> Don't you think it would be better to avoid the in code #ifdefs by doing some
>>> checks and defines in the header file and defining what's need to just use
>>> is_compat_task().
>>>
>>
>> Yes, might be...
>>
>>> Not sure 2 patches are needed for this either ......
>>>
>>
>> Well, I found this issue occasionally.
>
> I'm wondering what the symptoms are?
>
Size of struct autofs_v5_packet is 300 bytes for x86 and 304 bytes for x86_64.
Which means, that 32bit task can read more than size of autofs_v5_packet on 64bit kernel.
>> And, frankly speaking, it's not clear to me, whether this issue is important at all, so I wanted to clarify this first.
>> Thanks to O_DIRECT, the only way to catch the issue is to try to read more, than expected, in compat task (that's how I found it).
>
> Right, the O_DIRECT patch from Linus was expected to fix the structure
> alignment problem. The stuct field offsets are ok aren't they?
>
Yes, they are ok.
>> I don't see any other flaw so far. And if so, that, probably, we shouldn't care about the issue at all.
>> What do you think?
>
> If we are seeing hangs, incorrect struct fields or similar something
> should be done about it but if all is actually working ok then the
> O_DIRECT fix is doing it's job and further changes aren't necessary.
>
Well, yes. O_DIRECT fix covers the issue.
Ok then.
Thanks for the clarification!
> Ian
>
On 14/09/17 19:39, Stanislav Kinsburskiy wrote:
>
>
> 14.09.2017 13:29, Ian Kent пишет:
>> On 14/09/17 17:24, Stanislav Kinsburskiy wrote:
>>>
>>>
>>> 14.09.2017 02:38, Ian Kent пишет:
>>>> On 01/09/17 19:21, Stanislav Kinsburskiy wrote:
>>>>> Signed-off-by: Stanislav Kinsburskiy <[email protected]>
>>>>> ---
>>>>> fs/autofs4/autofs_i.h | 3 +++
>>>>> fs/autofs4/dev-ioctl.c | 3 +++
>>>>> fs/autofs4/inode.c | 4 +++-
>>>>> 3 files changed, 9 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/fs/autofs4/autofs_i.h b/fs/autofs4/autofs_i.h
>>>>> index 4737615..3da105f 100644
>>>>> --- a/fs/autofs4/autofs_i.h
>>>>> +++ b/fs/autofs4/autofs_i.h
>>>>> @@ -120,6 +120,9 @@ struct autofs_sb_info {
>>>>> struct list_head active_list;
>>>>> struct list_head expiring_list;
>>>>> struct rcu_head rcu;
>>>>> +#ifdef CONFIG_COMPAT
>>>>> + unsigned is32bit:1;
>>>>> +#endif
>>>>> };
>>>>>
>>>>> static inline struct autofs_sb_info *autofs4_sbi(struct super_block *sb)
>>>>> diff --git a/fs/autofs4/dev-ioctl.c b/fs/autofs4/dev-ioctl.c
>>>>> index b7c816f..467d6c4 100644
>>>>> --- a/fs/autofs4/dev-ioctl.c
>>>>> +++ b/fs/autofs4/dev-ioctl.c
>>>>> @@ -397,6 +397,9 @@ static int autofs_dev_ioctl_setpipefd(struct file *fp,
>>>>> sbi->pipefd = pipefd;
>>>>> sbi->pipe = pipe;
>>>>> sbi->catatonic = 0;
>>>>> +#ifdef CONFIG_COMPAT
>>>>> + sbi->is32bit = is_compat_task();
>>>>> +#endif
>>>>> }
>>>>> out:
>>>>> put_pid(new_pid);
>>>>> diff --git a/fs/autofs4/inode.c b/fs/autofs4/inode.c
>>>>> index 09e7d68..21d3c0b 100644
>>>>> --- a/fs/autofs4/inode.c
>>>>> +++ b/fs/autofs4/inode.c
>>>>> @@ -301,7 +301,9 @@ int autofs4_fill_super(struct super_block *s, void *data, int silent)
>>>>> } else {
>>>>> sbi->oz_pgrp = get_task_pid(current, PIDTYPE_PGID);
>>>>> }
>>>>> -
>>>>> +#ifdef CONFIG_COMPAT
>>>>> + sbi->is32bit = is_compat_task();
>>>>> +#endif
>>>>> if (autofs_type_trigger(sbi->type))
>>>>> __managed_dentry_set_managed(root);
>>>>>
>>>>>
>>>>
>>>> Not sure about this.
>>>>
>>>> Don't you think it would be better to avoid the in code #ifdefs by doing some
>>>> checks and defines in the header file and defining what's need to just use
>>>> is_compat_task().
>>>>
>>>
>>> Yes, might be...
>>>
>>>> Not sure 2 patches are needed for this either ......
>>>>
>>>
>>> Well, I found this issue occasionally.
>>
>> I'm wondering what the symptoms are?
>>
>
> Size of struct autofs_v5_packet is 300 bytes for x86 and 304 bytes for x86_64.
> Which means, that 32bit task can read more than size of autofs_v5_packet on 64bit kernel.
Are you sure?
Shouldn't that be a short read on the x86 side of a 4 bytes longer
structure on the x86_64 side.
I didn't think you could have a 64 bit client on a 32 bit kernel
so the converse (the read past end of struct) doesn't apply.
Ian
14.09.2017 13:45, Ian Kent пишет:
> On 14/09/17 19:39, Stanislav Kinsburskiy wrote:
>>
>>
>> 14.09.2017 13:29, Ian Kent пишет:
>>> On 14/09/17 17:24, Stanislav Kinsburskiy wrote:
>>>>
>>>>
>>>> 14.09.2017 02:38, Ian Kent пишет:
>>>>> On 01/09/17 19:21, Stanislav Kinsburskiy wrote:
>>>>>> Signed-off-by: Stanislav Kinsburskiy <[email protected]>
>>>>>> ---
>>>>>> fs/autofs4/autofs_i.h | 3 +++
>>>>>> fs/autofs4/dev-ioctl.c | 3 +++
>>>>>> fs/autofs4/inode.c | 4 +++-
>>>>>> 3 files changed, 9 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/fs/autofs4/autofs_i.h b/fs/autofs4/autofs_i.h
>>>>>> index 4737615..3da105f 100644
>>>>>> --- a/fs/autofs4/autofs_i.h
>>>>>> +++ b/fs/autofs4/autofs_i.h
>>>>>> @@ -120,6 +120,9 @@ struct autofs_sb_info {
>>>>>> struct list_head active_list;
>>>>>> struct list_head expiring_list;
>>>>>> struct rcu_head rcu;
>>>>>> +#ifdef CONFIG_COMPAT
>>>>>> + unsigned is32bit:1;
>>>>>> +#endif
>>>>>> };
>>>>>>
>>>>>> static inline struct autofs_sb_info *autofs4_sbi(struct super_block *sb)
>>>>>> diff --git a/fs/autofs4/dev-ioctl.c b/fs/autofs4/dev-ioctl.c
>>>>>> index b7c816f..467d6c4 100644
>>>>>> --- a/fs/autofs4/dev-ioctl.c
>>>>>> +++ b/fs/autofs4/dev-ioctl.c
>>>>>> @@ -397,6 +397,9 @@ static int autofs_dev_ioctl_setpipefd(struct file *fp,
>>>>>> sbi->pipefd = pipefd;
>>>>>> sbi->pipe = pipe;
>>>>>> sbi->catatonic = 0;
>>>>>> +#ifdef CONFIG_COMPAT
>>>>>> + sbi->is32bit = is_compat_task();
>>>>>> +#endif
>>>>>> }
>>>>>> out:
>>>>>> put_pid(new_pid);
>>>>>> diff --git a/fs/autofs4/inode.c b/fs/autofs4/inode.c
>>>>>> index 09e7d68..21d3c0b 100644
>>>>>> --- a/fs/autofs4/inode.c
>>>>>> +++ b/fs/autofs4/inode.c
>>>>>> @@ -301,7 +301,9 @@ int autofs4_fill_super(struct super_block *s, void *data, int silent)
>>>>>> } else {
>>>>>> sbi->oz_pgrp = get_task_pid(current, PIDTYPE_PGID);
>>>>>> }
>>>>>> -
>>>>>> +#ifdef CONFIG_COMPAT
>>>>>> + sbi->is32bit = is_compat_task();
>>>>>> +#endif
>>>>>> if (autofs_type_trigger(sbi->type))
>>>>>> __managed_dentry_set_managed(root);
>>>>>>
>>>>>>
>>>>>
>>>>> Not sure about this.
>>>>>
>>>>> Don't you think it would be better to avoid the in code #ifdefs by doing some
>>>>> checks and defines in the header file and defining what's need to just use
>>>>> is_compat_task().
>>>>>
>>>>
>>>> Yes, might be...
>>>>
>>>>> Not sure 2 patches are needed for this either ......
>>>>>
>>>>
>>>> Well, I found this issue occasionally.
>>>
>>> I'm wondering what the symptoms are?
>>>
>>
>> Size of struct autofs_v5_packet is 300 bytes for x86 and 304 bytes for x86_64.
>> Which means, that 32bit task can read more than size of autofs_v5_packet on 64bit kernel.
>
> Are you sure?
>
> Shouldn't that be a short read on the x86 side of a 4 bytes longer
> structure on the x86_64 side.
>
> I didn't think you could have a 64 bit client on a 32 bit kernel
> so the converse (the read past end of struct) doesn't apply.
>
Sorry for the confusion, I had to add brackets like this:
> Which means, that 32bit task can read more than size of autofs_v5_packet (on 64bit kernel).
IOW, 32bit task expects to read 300 bytes (size of struct autofs_v5_packet) while there are 304 bytes available on the "wire" from the 64bit kernel.
> Ian
>