Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752909AbcJJPKS convert rfc822-to-8bit (ORCPT ); Mon, 10 Oct 2016 11:10:18 -0400 Received: from mx1.redhat.com ([209.132.183.28]:58856 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752306AbcJJPKQ (ORCPT ); Mon, 10 Oct 2016 11:10:16 -0400 Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 10.0 \(3226\)) Subject: Re: [PATCHv2] ceph: Fix error handling in ceph_read_iter From: "Yan, Zheng" In-Reply-To: Date: Mon, 10 Oct 2016 23:09:52 +0800 Cc: Nikolay Borisov , ceph-devel , "linux-kernel@vger.kernel.org" Content-Transfer-Encoding: 8BIT Message-Id: 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> To: Ilya Dryomov X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Mon, 10 Oct 2016 15:09:57 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2004 Lines: 60 > On 10 Oct 2016, at 21:50, Ilya Dryomov wrote: > > 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? No. this change is obvious, what’s your concern? Regards Yan, Zheng > > Thanks, > > Ilya