* [PATCH] mm:bugfix check return value of ioremap_prot
@ 2018-08-02 7:37 chenjie6
2018-08-02 16:47 ` Yang Shi
0 siblings, 1 reply; 5+ messages in thread
From: chenjie6 @ 2018-08-02 7:37 UTC (permalink / raw)
To: linux-mm, tj; +Cc: akpm, lizefan, chen jie, chen jie
From: chen jie <chen jie@chenjie6@huwei.com>
ioremap_prot can return NULL which could lead to an oops
Signed-off-by: chen jie <chenjie6@huawei.com>
---
mm/memory.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/mm/memory.c b/mm/memory.c
index 7206a63..316c42e 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -4397,6 +4397,9 @@ int generic_access_phys(struct vm_area_struct *vma, unsigned long addr,
return -EINVAL;
maddr = ioremap_prot(phys_addr, PAGE_ALIGN(len + offset), prot);
+ if (!maddr)
+ return -ENOMEM;
+
if (write)
memcpy_toio(maddr + offset, buf, len);
else
--
1.8.3.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] mm:bugfix check return value of ioremap_prot
2018-08-02 7:37 [PATCH] mm:bugfix check return value of ioremap_prot chenjie6
@ 2018-08-02 16:47 ` Yang Shi
2018-08-02 21:02 ` Andrew Morton
0 siblings, 1 reply; 5+ messages in thread
From: Yang Shi @ 2018-08-02 16:47 UTC (permalink / raw)
To: chenjie6; +Cc: linux-mm, tj, Andrew Morton, lizefan, chen jie
On Thu, Aug 2, 2018 at 12:37 AM, <chenjie6@huawei.com> wrote:
> From: chen jie <chen jie@chenjie6@huwei.com>
>
> ioremap_prot can return NULL which could lead to an oops
What oops? You'd better to have the oops information in your commit log.
Thanks,
Yang
>
> Signed-off-by: chen jie <chenjie6@huawei.com>
> ---
> mm/memory.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/mm/memory.c b/mm/memory.c
> index 7206a63..316c42e 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -4397,6 +4397,9 @@ int generic_access_phys(struct vm_area_struct *vma, unsigned long addr,
> return -EINVAL;
>
> maddr = ioremap_prot(phys_addr, PAGE_ALIGN(len + offset), prot);
> + if (!maddr)
> + return -ENOMEM;
> +
> if (write)
> memcpy_toio(maddr + offset, buf, len);
> else
> --
> 1.8.3.4
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] mm:bugfix check return value of ioremap_prot
2018-08-02 16:47 ` Yang Shi
@ 2018-08-02 21:02 ` Andrew Morton
2018-08-03 12:47 ` Alexey Dobriyan
0 siblings, 1 reply; 5+ messages in thread
From: Andrew Morton @ 2018-08-02 21:02 UTC (permalink / raw)
To: Yang Shi; +Cc: chenjie6, linux-mm, tj, lizefan, chen jie, Alexey Dobriyan
On Thu, 2 Aug 2018 09:47:52 -0700 Yang Shi <shy828301@gmail.com> wrote:
> On Thu, Aug 2, 2018 at 12:37 AM, <chenjie6@huawei.com> wrote:
> > From: chen jie <chen jie@chenjie6@huwei.com>
> >
> > ioremap_prot can return NULL which could lead to an oops
>
> What oops? You'd better to have the oops information in your commit log.
Doesn't matter much - the code is clearly buggy.
Looking at the callers, I have suspicions about
fs/proc/base.c:environ_read(). It's assuming that access_remote_vm()
returns an errno. But it doesn't - it returns number of bytes copied.
Alexey, could you please take a look? While in there, I'd suggest
adding some return value documentation to __access_remote_vm() and
access_remote_vm(). Thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] mm:bugfix check return value of ioremap_prot
2018-08-02 21:02 ` Andrew Morton
@ 2018-08-03 12:47 ` Alexey Dobriyan
2018-08-03 23:05 ` Andrew Morton
0 siblings, 1 reply; 5+ messages in thread
From: Alexey Dobriyan @ 2018-08-03 12:47 UTC (permalink / raw)
To: Andrew Morton; +Cc: Yang Shi, chenjie6, linux-mm, tj, lizefan
On Thu, Aug 02, 2018 at 02:02:22PM -0700, Andrew Morton wrote:
> On Thu, 2 Aug 2018 09:47:52 -0700 Yang Shi <shy828301@gmail.com> wrote:
>
> > On Thu, Aug 2, 2018 at 12:37 AM, <chenjie6@huawei.com> wrote:
> > > From: chen jie <chen jie@chenjie6@huwei.com>
> > >
> > > ioremap_prot can return NULL which could lead to an oops
> >
> > What oops? You'd better to have the oops information in your commit log.
>
> Doesn't matter much - the code is clearly buggy.
>
> Looking at the callers, I have suspicions about
> fs/proc/base.c:environ_read(). It's assuming that access_remote_vm()
> returns an errno. But it doesn't - it returns number of bytes copied.
>
> Alexey, could you please take a look? While in there, I'd suggest
> adding some return value documentation to __access_remote_vm() and
> access_remote_vm(). Thanks.
This is true: remote VM accessors return number of bytes copied
but ->access returns len/-E. Returning "int" is deceptive.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] mm:bugfix check return value of ioremap_prot
2018-08-03 12:47 ` Alexey Dobriyan
@ 2018-08-03 23:05 ` Andrew Morton
0 siblings, 0 replies; 5+ messages in thread
From: Andrew Morton @ 2018-08-03 23:05 UTC (permalink / raw)
To: Alexey Dobriyan; +Cc: Yang Shi, chenjie6, linux-mm, tj, lizefan
On Fri, 3 Aug 2018 15:47:01 +0300 Alexey Dobriyan <adobriyan@gmail.com> wrote:
> On Thu, Aug 02, 2018 at 02:02:22PM -0700, Andrew Morton wrote:
> > On Thu, 2 Aug 2018 09:47:52 -0700 Yang Shi <shy828301@gmail.com> wrote:
> >
> > > On Thu, Aug 2, 2018 at 12:37 AM, <chenjie6@huawei.com> wrote:
> > > > From: chen jie <chen jie@chenjie6@huwei.com>
> > > >
> > > > ioremap_prot can return NULL which could lead to an oops
> > >
> > > What oops? You'd better to have the oops information in your commit log.
> >
> > Doesn't matter much - the code is clearly buggy.
> >
> > Looking at the callers, I have suspicions about
> > fs/proc/base.c:environ_read(). It's assuming that access_remote_vm()
> > returns an errno. But it doesn't - it returns number of bytes copied.
> >
> > Alexey, could you please take a look? While in there, I'd suggest
> > adding some return value documentation to __access_remote_vm() and
> > access_remote_vm(). Thanks.
>
> This is true: remote VM accessors return number of bytes copied
> but ->access returns len/-E. Returning "int" is deceptive.
It's more than deceptive - it's flat-out buggy for >4G copy attempts.
And highly dubious for 2G-4G copies, where it might return a negative
int.
I suppose that access_remote_vm() should strictly return a ptrdiff_t,
but we hardly ever use that. size_t will do.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-08-03 23:06 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-02 7:37 [PATCH] mm:bugfix check return value of ioremap_prot chenjie6
2018-08-02 16:47 ` Yang Shi
2018-08-02 21:02 ` Andrew Morton
2018-08-03 12:47 ` Alexey Dobriyan
2018-08-03 23:05 ` Andrew Morton
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.