| Message ID | alpine.DEB.2.00.1805102009380.10896@tp.orcam.me.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Alexander, On Tue, May 15, 2018 at 11:32:45PM +0100, Maciej W. Rozycki wrote: > The regset API documented in <linux/regset.h> defines -ENODEV as the > result of the `->active' handler to be used where the feature requested > is not available on the hardware found. However code handling core file > note generation in `fill_thread_core_info' interpretes any non-zero > result from the `->active' handler as the regset requested being active. > Consequently processing continues (and hopefully gracefully fails later > on) rather than being abandoned right away for the regset requested. > > Fix the problem then by making the code proceed only if a positive > result is returned from the `->active' handler. > > Cc: stable@vger.kernel.org # 2.6.25+ > Fixes: 4206d3aa1978 ("elf core dump: notes user_regset") > Signed-off-by: Maciej W. Rozycki <macro@mips.com> <snip> > --- linux-jhogan-test.orig/fs/binfmt_elf.c 2018-03-21 17:14:55.000000000 +0000 > +++ linux-jhogan-test/fs/binfmt_elf.c 2018-05-09 23:25:50.742255000 +0100 > @@ -1739,7 +1739,7 @@ static int fill_thread_core_info(struct > const struct user_regset *regset = &view->regsets[i]; > do_thread_regset_writeback(t->task, regset); > if (regset->core_note_type && regset->get && > - (!regset->active || regset->active(t->task, regset))) { > + (!regset->active || regset->active(t->task, regset) > 0)) { > int ret; > size_t size = regset_size(t->task, regset); > void *data = kmalloc(size, GFP_KERNEL); > This looks obviously right to me, although I don't think it affects anything until commit 25847fb195ae ("powerpc/ptrace: Enable support for NT_PPC_CGPR") in v4.8-rc1 & even then not in a harmful way so I'd drop the stable tag. You show up as maintainer for fs/binfmt_elf.c though, so before I go applying this to mips-next does it look good to you? Thanks, Paul
Hi Paul, > > --- linux-jhogan-test.orig/fs/binfmt_elf.c 2018-03-21 17:14:55.000000000 +0000 > > +++ linux-jhogan-test/fs/binfmt_elf.c 2018-05-09 23:25:50.742255000 +0100 > > @@ -1739,7 +1739,7 @@ static int fill_thread_core_info(struct > > const struct user_regset *regset = &view->regsets[i]; > > do_thread_regset_writeback(t->task, regset); > > if (regset->core_note_type && regset->get && > > - (!regset->active || regset->active(t->task, regset))) { > > + (!regset->active || regset->active(t->task, regset) > 0)) { > > int ret; > > size_t size = regset_size(t->task, regset); > > void *data = kmalloc(size, GFP_KERNEL); > > > > This looks obviously right to me, although I don't think it affects > anything until commit 25847fb195ae ("powerpc/ptrace: Enable support for > NT_PPC_CGPR") in v4.8-rc1 & even then not in a harmful way so I'd drop > the stable tag. I'm fine with dropping the tag FWIW. Thanks for taking care of my patch. Maciej
Index: linux-jhogan-test/fs/binfmt_elf.c =================================================================== --- linux-jhogan-test.orig/fs/binfmt_elf.c 2018-03-21 17:14:55.000000000 +0000 +++ linux-jhogan-test/fs/binfmt_elf.c 2018-05-09 23:25:50.742255000 +0100 @@ -1739,7 +1739,7 @@ static int fill_thread_core_info(struct const struct user_regset *regset = &view->regsets[i]; do_thread_regset_writeback(t->task, regset); if (regset->core_note_type && regset->get && - (!regset->active || regset->active(t->task, regset))) { + (!regset->active || regset->active(t->task, regset) > 0)) { int ret; size_t size = regset_size(t->task, regset); void *data = kmalloc(size, GFP_KERNEL);
The regset API documented in <linux/regset.h> defines -ENODEV as the result of the `->active' handler to be used where the feature requested is not available on the hardware found. However code handling core file note generation in `fill_thread_core_info' interpretes any non-zero result from the `->active' handler as the regset requested being active. Consequently processing continues (and hopefully gracefully fails later on) rather than being abandoned right away for the regset requested. Fix the problem then by making the code proceed only if a positive result is returned from the `->active' handler. Cc: stable@vger.kernel.org # 2.6.25+ Fixes: 4206d3aa1978 ("elf core dump: notes user_regset") Signed-off-by: Maciej W. Rozycki <macro@mips.com> --- Hi, Overall we could also use the count returned by ->active to limit the size of data requested, i.e. something along the lines of: ssize_t size; if (regset->active) size = regset->active(t->task, regset); else size = regset_size(t->task, regset); if (size > 0) { ... however that would be an optimisation that belongs to a separate change, which (due to the need to unload stuff I have in progress already) I am not going to make at this point. Perhaps someone else would be willing to pick up the idea. Anyway, please apply. Maciej --- fs/binfmt_elf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) linux-elf-core-regset-active.diff