Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752590AbcJJNup (ORCPT ); Mon, 10 Oct 2016 09:50:45 -0400 Received: from mail-qk0-f196.google.com ([209.85.220.196]:35294 "EHLO mail-qk0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752215AbcJJNuo (ORCPT ); Mon, 10 Oct 2016 09:50:44 -0400 MIME-Version: 1.0 In-Reply-To: <57FB93ED.5030602@kyup.com> References: <1476103098-2925-1-git-send-email-kernel@kyup.com> <1476104199-3259-1-git-send-email-kernel@kyup.com> <530F1E79-6653-45F5-BBC6-B1D9F9005D14@redhat.com> <57FB93ED.5030602@kyup.com> From: Ilya Dryomov Date: Mon, 10 Oct 2016 15:50:42 +0200 Message-ID: Subject: Re: [PATCHv2] ceph: Fix error handling in ceph_read_iter To: Nikolay Borisov Cc: "Yan, Zheng" , ceph-devel , "linux-kernel@vger.kernel.org" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by mail.home.local id u9ADopS9003202 Content-Length: 1795 Lines: 51 On Mon, Oct 10, 2016 at 3:13 PM, Nikolay Borisov wrote: > > > On 10/10/2016 04:11 PM, Yan, Zheng wrote: >> >>> On 10 Oct 2016, at 20:56, Nikolay Borisov wrote: >>> >>> In case __ceph_do_getattr returns an error and the retry_op in >>> ceph_read_iter is not READ_INLINE, then it's possible to invoke >>> __free_page on a page which is NULL, this naturally leads to a crash. >>> This can happen when, for example, a process waiting on a MDS reply >>> receives sigterm. >>> >>> Fix this by explicitly checking whether the page is set or not. >>> >>> Signed-off-by: Nikolay Borisov >>> Link: http://www.spinics.net/lists/ceph-users/msg31592.html >>> --- >>> >>> Inverted the condition, so resending with correct condition >>> this time. >>> >>> fs/ceph/file.c | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/fs/ceph/file.c b/fs/ceph/file.c >>> index 3c68e6aee2f0..7413313ae6c8 100644 >>> --- a/fs/ceph/file.c >>> +++ b/fs/ceph/file.c >>> @@ -929,7 +929,8 @@ again: >>> statret = __ceph_do_getattr(inode, page, >>> CEPH_STAT_CAP_INLINE_DATA, !!page); >>> if (statret < 0) { >>> - __free_page(page); >>> + if (page) >>> + __free_page(page); >>> if (statret == -ENODATA) { >>> BUG_ON(retry_op != READ_INLINE); >>> goto again; >>> — >> Reviewed-by: Yan, Zheng > > I believe this needs to also be tagged as stable. To whomever is going > to merge it: can you please do that? I'll do that. Zheng, do you see any other issues with the killable wait here? Thanks, Ilya