Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1731994 > unrolled thread
| Started by | Ian Kent <raven@themaw.net> |
|---|---|
| First post | 2017-09-14 02:40 +0200 |
| Last post | 2017-09-14 13:50 +0200 |
| Articles | 3 — 1 participant |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: [RFC PATCH 1/2] autofs: set compat flag on sbi when daemon uses 32bit addressation Ian Kent <raven@themaw.net> - 2017-09-14 02:40 +0200
Re: [RFC PATCH 1/2] autofs: set compat flag on sbi when daemon uses 32bit addressation Ian Kent <raven@themaw.net> - 2017-09-14 13:30 +0200
Re: [RFC PATCH 1/2] autofs: set compat flag on sbi when daemon uses 32bit addressation Ian Kent <raven@themaw.net> - 2017-09-14 13:50 +0200
| From | Ian Kent <raven@themaw.net> |
|---|---|
| Date | 2017-09-14 02:40 +0200 |
| Subject | Re: [RFC PATCH 1/2] autofs: set compat flag on sbi when daemon uses 32bit addressation |
| Message-ID | <upqmd-64c-7@gated-at.bofh.it> |
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
[toc] | [next] | [standalone]
| From | Ian Kent <raven@themaw.net> |
|---|---|
| Date | 2017-09-14 13:30 +0200 |
| Message-ID | <upAvf-4nM-1@gated-at.bofh.it> |
| In reply to | #1731994 |
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
[toc] | [prev] | [next] | [standalone]
| From | Ian Kent <raven@themaw.net> |
|---|---|
| Date | 2017-09-14 13:50 +0200 |
| Message-ID | <upAOC-4uy-21@gated-at.bofh.it> |
| In reply to | #1732222 |
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
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web