From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-dm3nam03on0118.outbound.protection.outlook.com ([104.47.41.118]:43271 "EHLO NAM03-DM3-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S932446AbeF2RNg (ORCPT ); Fri, 29 Jun 2018 13:13:36 -0400 Date: Fri, 29 Jun 2018 10:13:30 -0700 From: Paul Burton To: Alexander Viro Cc: "Maciej W. Rozycki" , James Hogan , Ralf Baechle , linux-fsdevel@vger.kernel.org, linux-mips@linux-mips.org, linux-kernel@vger.kernel.org, stable@vger.kernel.org Subject: Re: [PATCH 1/3] binfmt_elf: Respect error return from `regset->active' Message-ID: <20180629171330.4giikc5x2cbxxuyc@pburton-laptop> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Sender: linux-fsdevel-owner@vger.kernel.org List-ID: Hi Alexander, On Tue, May 15, 2018 at 11:32:45PM +0100, Maciej W. Rozycki wrote: > The regset API documented in 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 > --- 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