[RFC PATCH 0/2] autofs: fix autofs_v5_packet dlivery in compat mode

[RFC PATCH 0/2] autofs: fix autofs_v5_packet dlivery in compat mode

[Stanislav Kinsburskiy]

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(-)

[RFC PATCH 0/2] autofs: fix autofs_v5_packet dlivery in compat mode

[Stanislav Kinsburskiy]

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(-)
--
To unsubscribe from this list: send the line "unsubscribe autofs" in

[RFC PATCH 1/2] autofs: set compat flag on sbi when daemon uses 32bit addressation

[Stanislav Kinsburskiy]

Signed-off-by: Stanislav Kinsburskiy <skinsbursky@virtuozzo.com>
---
 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);
 

[RFC PATCH 1/2] autofs: set compat flag on sbi when daemon uses 32bit addressation

[Stanislav Kinsburskiy]

Signed-off-by: Stanislav Kinsburskiy <skinsbursky@virtuozzo.com>
---
 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);
 

--
To unsubscribe from this list: send the line "unsubscribe autofs" in

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

[Stanislav Kinsburskiy]

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 <ldv@altlinux.org>
Signed-off-by: Stanislav Kinsburskiy <skinsbursky@virtuozzo.com>
---
 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;

Re: [RFC PATCH 0/2] autofs: fix autofs_v5_packet dlivery in compat mode

[Ian Kent]

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(-)
> 

Re: [RFC PATCH 0/2] autofs: fix autofs_v5_packet dlivery in compat mode

[Ian Kent]

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(-)
> 

--
To unsubscribe from this list: send the line "unsubscribe autofs" in

Re: [RFC PATCH 1/2] autofs: set compat flag on sbi when daemon uses 32bit addressation

[Ian Kent]

On 01/09/17 19:21, Stanislav Kinsburskiy wrote:
> Signed-off-by: Stanislav Kinsburskiy <skinsbursky@virtuozzo.com>
> ---
>  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

Re: [RFC PATCH 1/2] autofs: set compat flag on sbi when daemon uses 32bit addressation

[Ian Kent]

On 01/09/17 19:21, Stanislav Kinsburskiy wrote:
> Signed-off-by: Stanislav Kinsburskiy <skinsbursky@virtuozzo.com>
> ---
>  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
--
To unsubscribe from this list: send the line "unsubscribe autofs" in

Re: [RFC PATCH 1/2] autofs: set compat flag on sbi when daemon uses 32bit addressation

[Stanislav Kinsburskiy]

14.09.2017 02:38, Ian Kent пишет:
> On 01/09/17 19:21, Stanislav Kinsburskiy wrote:
>> Signed-off-by: Stanislav Kinsburskiy <skinsbursky@virtuozzo.com>
>> ---
>>  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
> 


 

Re: [RFC PATCH 1/2] autofs: set compat flag on sbi when daemon uses 32bit addressation

[Stanislav Kinsburskiy]

14.09.2017 02:38, Ian Kent пишет:
> On 01/09/17 19:21, Stanislav Kinsburskiy wrote:
>> Signed-off-by: Stanislav Kinsburskiy <skinsbursky@virtuozzo.com>
>> ---
>>  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
> 


 
--
To unsubscribe from this list: send the line "unsubscribe autofs" in

Re: [RFC PATCH 1/2] autofs: set compat flag on sbi when daemon uses 32bit addressation

[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 <skinsbursky@virtuozzo.com>
>>> ---
>>>  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

Re: [RFC PATCH 1/2] autofs: set compat flag on sbi when daemon uses 32bit addressation

[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 <skinsbursky@virtuozzo.com>
>>> ---
>>>  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
--
To unsubscribe from this list: send the line "unsubscribe autofs" in

Re: [RFC PATCH 1/2] autofs: set compat flag on sbi when daemon uses 32bit addressation

[Stanislav Kinsburskiy]

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 <skinsbursky@virtuozzo.com>
>>>> ---
>>>>  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
> 

Re: [RFC PATCH 1/2] autofs: set compat flag on sbi when daemon uses 32bit addressation

[Stanislav Kinsburskiy]

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 <skinsbursky@virtuozzo.com>
>>>> ---
>>>>  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
> 
--
To unsubscribe from this list: send the line "unsubscribe autofs" in

Re: [RFC PATCH 1/2] autofs: set compat flag on sbi when daemon uses 32bit addressation

[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 <skinsbursky@virtuozzo.com>
>>>>> ---
>>>>>  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

Re: [RFC PATCH 1/2] autofs: set compat flag on sbi when daemon uses 32bit addressation

[Stanislav Kinsburskiy]

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 <skinsbursky@virtuozzo.com>
>>>>>> ---
>>>>>>  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
> 

Re: [RFC PATCH 1/2] autofs: set compat flag on sbi when daemon uses 32bit addressation

[Stanislav Kinsburskiy]

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 <skinsbursky@virtuozzo.com>
>>>>>> ---
>>>>>>  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
> 
--
To unsubscribe from this list: send the line "unsubscribe autofs" in