From: HATAYAMA Daisuke <d.hatayama@jp.fujitsu.com>
To: Alexey Dobriyan <adobriyan@gmail.com>
Cc: kexec@lists.infradead.org,
Linux Kernel <linux-kernel@vger.kernel.org>,
Jan Willeke <willeke@de.ibm.com>,
Michael Holzheu <holzheu@linux.vnet.ibm.com>,
"David S. Miller" <davem@davemloft.net>,
Vivek Goyal <vgoyal@redhat.com>
Subject: Re: mmap for /proc/vmcore broken since 3.12-rc1
Date: Mon, 14 Oct 2013 13:52:40 +0900 [thread overview]
Message-ID: <525B7898.6010802@jp.fujitsu.com> (raw)
In-Reply-To: <20131012203250.GA3458@p183.telecom.by>
(2013/10/13 5:32), Alexey Dobriyan wrote:
> On Wed, Oct 09, 2013 at 07:14:55PM +0900, HATAYAMA Daisuke wrote:
>> Hello,
>>
>> (2013/10/08 21:49), Alexey Dobriyan wrote:
>>> On Mon, Oct 7, 2013 at 5:42 AM, HATAYAMA Daisuke
>>> <d.hatayama@jp.fujitsu.com> wrote:
>>>
>>>> +static unsigned long
>>>> +get_unmapped_area_vmcore(struct file *filp, unsigned long addr,
>>>> + unsigned long len, unsigned long pgoff,
>>>> + unsigned long flags)
>>>> +{
>>>> +#ifdef CONFIG_MMU
>>>> + return current->mm->get_unmapped_area(filp, addr, len, pgoff,
>>>> flags);
>>>> +#else
>>>> + return -EIO;
>>>> +#endif
>>>> +}
>>>> +
>>>> static const struct file_operations proc_vmcore_operations = {
>>>> .read = read_vmcore,
>>>> .llseek = default_llseek,
>>>> .mmap = mmap_vmcore,
>>>> + .get_unmapped_area = get_unmapped_area_vmcore,
>>>
>>> I think current->mm->get_unmapped_area should be used by core proc code.
>>
>> What do you actually suggest here? You mean moving this code in proc code?
>> I don't think you suggest so.
>
> Please, try this patch, I don't have kexec setup handy.
>
> --- a/fs/proc/inode.c
> +++ b/fs/proc/inode.c
> @@ -291,7 +291,11 @@ static unsigned long proc_reg_get_unmapped_area(struct file *file, unsigned long
> int rv = -EIO;
> unsigned long (*get_unmapped_area)(struct file *, unsigned long, unsigned long, unsigned long, unsigned long);
> if (use_pde(pde)) {
> - get_unmapped_area = pde->proc_fops->get_unmapped_area;
> + get_unmapped_area = current->mm->get_unmapped_area;
> +#ifdef CONFIG_MMU
> + if (pde->proc_fops->get_unmapped_area)
> + get_unmapped_area = pde->proc_fops->get_unmapped_area;
> +#endif
> if (get_unmapped_area)
> rv = get_unmapped_area(file, orig_addr, len, pgoff, flags);
> unuse_pde(pde);
>
Slight modification to #ifdef ...
get_unmapped_area = NULL;
#ifdef CONFIG_MMU
get_unmapped_area = current->mm->get_unmapped_area
#endif
if (pde->proc_fops->get_unmapped_area)
get_unmapped_area = pde->proc_fops->get_unmapped_area;
And, I found the bug. The variable rv should have been defined as unsigned
long. sizeof(int) is 4 bytes but sizeof(long) is 8 bytes at least on x86_64.
The reason why returned value looked like kernel virtual address was due to
signed extension performed during conversion from negative 32-bit signed
integer to 64-bit unsigned long integer.
Hmm, I first checked signature of related functions but overlooked...
Anyway, I'll post fixing patch soon.
--
Thanks.
HATAYAMA, Daisuke
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
WARNING: multiple messages have this Message-ID (diff)
From: HATAYAMA Daisuke <d.hatayama@jp.fujitsu.com>
To: Alexey Dobriyan <adobriyan@gmail.com>
Cc: Michael Holzheu <holzheu@linux.vnet.ibm.com>,
"David S. Miller" <davem@davemloft.net>,
Vivek Goyal <vgoyal@redhat.com>, Jan Willeke <willeke@de.ibm.com>,
Linux Kernel <linux-kernel@vger.kernel.org>,
kexec@lists.infradead.org
Subject: Re: mmap for /proc/vmcore broken since 3.12-rc1
Date: Mon, 14 Oct 2013 13:52:40 +0900 [thread overview]
Message-ID: <525B7898.6010802@jp.fujitsu.com> (raw)
In-Reply-To: <20131012203250.GA3458@p183.telecom.by>
(2013/10/13 5:32), Alexey Dobriyan wrote:
> On Wed, Oct 09, 2013 at 07:14:55PM +0900, HATAYAMA Daisuke wrote:
>> Hello,
>>
>> (2013/10/08 21:49), Alexey Dobriyan wrote:
>>> On Mon, Oct 7, 2013 at 5:42 AM, HATAYAMA Daisuke
>>> <d.hatayama@jp.fujitsu.com> wrote:
>>>
>>>> +static unsigned long
>>>> +get_unmapped_area_vmcore(struct file *filp, unsigned long addr,
>>>> + unsigned long len, unsigned long pgoff,
>>>> + unsigned long flags)
>>>> +{
>>>> +#ifdef CONFIG_MMU
>>>> + return current->mm->get_unmapped_area(filp, addr, len, pgoff,
>>>> flags);
>>>> +#else
>>>> + return -EIO;
>>>> +#endif
>>>> +}
>>>> +
>>>> static const struct file_operations proc_vmcore_operations = {
>>>> .read = read_vmcore,
>>>> .llseek = default_llseek,
>>>> .mmap = mmap_vmcore,
>>>> + .get_unmapped_area = get_unmapped_area_vmcore,
>>>
>>> I think current->mm->get_unmapped_area should be used by core proc code.
>>
>> What do you actually suggest here? You mean moving this code in proc code?
>> I don't think you suggest so.
>
> Please, try this patch, I don't have kexec setup handy.
>
> --- a/fs/proc/inode.c
> +++ b/fs/proc/inode.c
> @@ -291,7 +291,11 @@ static unsigned long proc_reg_get_unmapped_area(struct file *file, unsigned long
> int rv = -EIO;
> unsigned long (*get_unmapped_area)(struct file *, unsigned long, unsigned long, unsigned long, unsigned long);
> if (use_pde(pde)) {
> - get_unmapped_area = pde->proc_fops->get_unmapped_area;
> + get_unmapped_area = current->mm->get_unmapped_area;
> +#ifdef CONFIG_MMU
> + if (pde->proc_fops->get_unmapped_area)
> + get_unmapped_area = pde->proc_fops->get_unmapped_area;
> +#endif
> if (get_unmapped_area)
> rv = get_unmapped_area(file, orig_addr, len, pgoff, flags);
> unuse_pde(pde);
>
Slight modification to #ifdef ...
get_unmapped_area = NULL;
#ifdef CONFIG_MMU
get_unmapped_area = current->mm->get_unmapped_area
#endif
if (pde->proc_fops->get_unmapped_area)
get_unmapped_area = pde->proc_fops->get_unmapped_area;
And, I found the bug. The variable rv should have been defined as unsigned
long. sizeof(int) is 4 bytes but sizeof(long) is 8 bytes at least on x86_64.
The reason why returned value looked like kernel virtual address was due to
signed extension performed during conversion from negative 32-bit signed
integer to 64-bit unsigned long integer.
Hmm, I first checked signature of related functions but overlooked...
Anyway, I'll post fixing patch soon.
--
Thanks.
HATAYAMA, Daisuke
next prev parent reply other threads:[~2013-10-14 4:57 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-10-02 12:03 mmap for /proc/vmcore broken since 3.12-rc1 Michael Holzheu
2013-10-02 12:03 ` Michael Holzheu
2013-10-03 6:12 ` HATAYAMA Daisuke
2013-10-03 6:12 ` HATAYAMA Daisuke
2013-10-07 2:42 ` HATAYAMA Daisuke
2013-10-07 2:42 ` HATAYAMA Daisuke
2013-10-08 12:49 ` Alexey Dobriyan
2013-10-08 12:49 ` Alexey Dobriyan
2013-10-09 10:14 ` HATAYAMA Daisuke
2013-10-09 10:14 ` HATAYAMA Daisuke
2013-10-12 20:32 ` Alexey Dobriyan
2013-10-12 20:32 ` Alexey Dobriyan
2013-10-14 4:52 ` HATAYAMA Daisuke [this message]
2013-10-14 4:52 ` HATAYAMA Daisuke
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=525B7898.6010802@jp.fujitsu.com \
--to=d.hatayama@jp.fujitsu.com \
--cc=adobriyan@gmail.com \
--cc=davem@davemloft.net \
--cc=holzheu@linux.vnet.ibm.com \
--cc=kexec@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=vgoyal@redhat.com \
--cc=willeke@de.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.