| Message ID | 1536100702-28706-1-git-send-email-asmadeus@codewreck.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] proc/kcore: fix invalid memory access in multi-page read optimization | expand |
On Wed, Sep 05, 2018 at 12:38:22AM +0200, Dominique Martinet wrote: > The 'm' kcore_list item could point to kclist_head, and it is incorrect to > look at m->addr / m->size in this case. > There is no choice but to run through the list of entries for every address > if we did not find any entry in the previous iteration > > Reset 'm' to NULL in that case at Omar Sandoval's suggestion. > > Fixes: bf991c2231117 ("proc/kcore: optimize multiple page reads") Reviewed-by: Omar Sandoval <osandov@fb.com> Thanks again for catching this! > Signed-off-by: Dominique Martinet <asmadeus@codewreck.org> > --- > > Sorry, resent v2 because From didn't match sob tag > > fs/proc/kcore.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/fs/proc/kcore.c b/fs/proc/kcore.c > index ad72261ee3fe..578926032880 100644 > --- a/fs/proc/kcore.c > +++ b/fs/proc/kcore.c > @@ -464,6 +464,7 @@ read_kcore(struct file *file, char __user *buffer, size_t buflen, loff_t *fpos) > ret = -EFAULT; > goto out; > } > + m = NULL; > } else if (m->type == KCORE_VMALLOC) { > vread(buf, (char *)start, tsz); > /* we have to zero-fill user buffer even if no read */ > -- > 2.17.1 >
On Wed, Sep 5, 2018 at 4:11 AM, Omar Sandoval <osandov@osandov.com> wrote: > On Wed, Sep 05, 2018 at 12:38:22AM +0200, Dominique Martinet wrote: >> The 'm' kcore_list item could point to kclist_head, and it is incorrect to >> look at m->addr / m->size in this case. >> There is no choice but to run through the list of entries for every address >> if we did not find any entry in the previous iteration >> >> Reset 'm' to NULL in that case at Omar Sandoval's suggestion. >> >> Fixes: bf991c2231117 ("proc/kcore: optimize multiple page reads") > > Reviewed-by: Omar Sandoval <osandov@fb.com> > > Thanks again for catching this! > >> Signed-off-by: Dominique Martinet <asmadeus@codewreck.org> >> --- >> >> Sorry, resent v2 because From didn't match sob tag >> >> fs/proc/kcore.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/fs/proc/kcore.c b/fs/proc/kcore.c >> index ad72261ee3fe..578926032880 100644 >> --- a/fs/proc/kcore.c >> +++ b/fs/proc/kcore.c >> @@ -464,6 +464,7 @@ read_kcore(struct file *file, char __user *buffer, size_t buflen, loff_t *fpos) >> ret = -EFAULT; >> goto out; >> } >> + m = NULL; >> } else if (m->type == KCORE_VMALLOC) { >> vread(buf, (char *)start, tsz); >> /* we have to zero-fill user buffer even if no read */ >> -- >> 2.17.1 Looks sane to me, so: Reviewed-by: Bhupesh Sharma <bhsharma@redhat.com> Thanks.
On Wed, 5 Sep 2018 00:38:22 +0200 Dominique Martinet <asmadeus@codewreck.org> wrote: > The 'm' kcore_list item could point to kclist_head, and it is incorrect to > look at m->addr / m->size in this case. > There is no choice but to run through the list of entries for every address > if we did not find any entry in the previous iteration > > Reset 'm' to NULL in that case at Omar Sandoval's suggestion. > > ... > > --- a/fs/proc/kcore.c > +++ b/fs/proc/kcore.c > @@ -464,6 +464,7 @@ read_kcore(struct file *file, char __user *buffer, size_t buflen, loff_t *fpos) > ret = -EFAULT; > goto out; > } > + m = NULL; > } else if (m->type == KCORE_VMALLOC) { > vread(buf, (char *)start, tsz); > /* we have to zero-fill user buffer even if no read */ lgtm. Let's add a nice little why-were-doing-this? --- a/fs/proc/kcore.c~proc-kcore-fix-invalid-memory-access-in-multi-page-read-optimization-v3-fix +++ a/fs/proc/kcore.c @@ -464,7 +464,7 @@ read_kcore(struct file *file, char __use ret = -EFAULT; goto out; } - m = NULL; + m = NULL; /* skip the list anchor */ } else if (m->type == KCORE_VMALLOC) { vread(buf, (char *)start, tsz); /* we have to zero-fill user buffer even if no read */
Andrew Morton wrote on Wed, Sep 05, 2018: > lgtm. Let's add a nice little why-were-doing-this? Sure, thank you. > --- a/fs/proc/kcore.c~proc-kcore-fix-invalid-memory-access-in-multi-page-read-optimization-v3-fix > +++ a/fs/proc/kcore.c > @@ -464,7 +464,7 @@ read_kcore(struct file *file, char __use > ret = -EFAULT; > goto out; > } > - m = NULL; > + m = NULL; /* skip the list anchor */ > } else if (m->type == KCORE_VMALLOC) { > vread(buf, (char *)start, tsz); > /* we have to zero-fill user buffer even if no read */
diff --git a/fs/proc/kcore.c b/fs/proc/kcore.c index ad72261ee3fe..578926032880 100644 --- a/fs/proc/kcore.c +++ b/fs/proc/kcore.c @@ -464,6 +464,7 @@ read_kcore(struct file *file, char __user *buffer, size_t buflen, loff_t *fpos) ret = -EFAULT; goto out; } + m = NULL; } else if (m->type == KCORE_VMALLOC) { vread(buf, (char *)start, tsz); /* we have to zero-fill user buffer even if no read */
The 'm' kcore_list item could point to kclist_head, and it is incorrect to look at m->addr / m->size in this case. There is no choice but to run through the list of entries for every address if we did not find any entry in the previous iteration Reset 'm' to NULL in that case at Omar Sandoval's suggestion. Fixes: bf991c2231117 ("proc/kcore: optimize multiple page reads") Signed-off-by: Dominique Martinet <asmadeus@codewreck.org> --- Sorry, resent v2 because From didn't match sob tag fs/proc/kcore.c | 1 + 1 file changed, 1 insertion(+)