From patchwork Thu Apr 26 16:11:57 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ilya Dryomov X-Patchwork-Id: 10366247 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 70DB26032C for ; Thu, 26 Apr 2018 16:12:29 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 62B6128CC4 for ; Thu, 26 Apr 2018 16:12:29 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 6085929185; Thu, 26 Apr 2018 16:12:29 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.8 required=2.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI, T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 88B4D29151 for ; Thu, 26 Apr 2018 16:12:03 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756754AbeDZQMB (ORCPT ); Thu, 26 Apr 2018 12:12:01 -0400 Received: from mail-it0-f68.google.com ([209.85.214.68]:36155 "EHLO mail-it0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756605AbeDZQL6 (ORCPT ); Thu, 26 Apr 2018 12:11:58 -0400 Received: by mail-it0-f68.google.com with SMTP id e20-v6so24466533itc.1 for ; Thu, 26 Apr 2018 09:11:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=abET8FaYN0ylQNtPnshDeVq/NtlGzWwrdq4IpfcCc3o=; b=NHCa5rbC7i7lYypJHj1AdvOejOwpmp6w+r15E2MALD9RvRbSkKPlrvz/9Wc/zUrpY2 HETn7/HSYeX8d1MeNPTp1s5BTeljkTwcaYBtwpMb5WGlmV/+dUaO7CejEQqRu4nTEcbZ Q5IkIuYp1R8E8Ge+D1UNjln4VhqlGw4w1L1SUQN2yISGI5LW4z+Bd82rTVHzZpVS/O7c JK812HGxquzssfGVaUDkEZY50ivLpSDR3n6VvNokeh8x2Qc8s5k8WsAMEfaYHK2/PvAu RbwldLILQZC6X1RbT0vIhvj34JGvHQB25/aF1GonMHcD80PtmqA8uxaCzPNa5XaD3/At 19Fw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=abET8FaYN0ylQNtPnshDeVq/NtlGzWwrdq4IpfcCc3o=; b=kHGOW640ohx0QNlz4VoDY6B1GHOl/LLV6025wko7UKj4exH30wEQRrBfCjd6ZlrO7S RGGveRbw85/Va2EoVorKeW5kpdbm3BOCmbGppsou9NY9CGMberJM7rNa3CrwoOktB1DJ sogurPVNgouWNOpnNVxKNySgrs+vHF1TmJWfhGwsv0wIFz1zxHjPmJIouyeXZbPKJWDc 50w3TblBeLVxb+LEj9faw+LuI8Yr12zgZtM7o4rhXOOWmeOg0FLKzdMQ1hBJ5usDc8vC WU734LqDOdqlIpUeZf0UQcfW933NlKklSIbG5aBJDSpIJ/u15973Z7J2+Nr2jYfB4PMD Z9Lw== X-Gm-Message-State: ALQs6tBV8K+8s3WHOP/MzyPbvJFcYXeykSl7QvjrSSNrmLMewBL5df8i 1tXvkWnqPWWh6E+n3EkRYHBwAbtGjnncVMajZfI= X-Google-Smtp-Source: AB8JxZoGS677fKUCPgsvf51J9EDrcKCFuNMZ75VlHWLTcgO1orQZp0KhuX8ir71374amZwNWIvnpB8mXNgvfnU+SVmE= X-Received: by 2002:a24:9710:: with SMTP id k16-v6mr5123790ite.150.1524759117598; Thu, 26 Apr 2018 09:11:57 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a02:b4fb:0:0:0:0:0 with HTTP; Thu, 26 Apr 2018 09:11:57 -0700 (PDT) In-Reply-To: References: <1524652133-7432-1-git-send-email-idryomov@gmail.com> From: Ilya Dryomov Date: Thu, 26 Apr 2018 18:11:57 +0200 Message-ID: Subject: Re: [PATCH] libceph: validate con->state at the top of try_write() To: Jason Dillaman Cc: ceph-devel Sender: ceph-devel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: ceph-devel@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Wed, Apr 25, 2018 at 5:44 PM, Jason Dillaman wrote: > On Wed, Apr 25, 2018 at 6:28 AM, Ilya Dryomov wrote: >> ceph_con_workfn() validates con->state before calling try_read() and >> then try_write(). However, try_read() temporarily releases con->mutex, >> notably in process_message() and ceph_con_in_msg_alloc(), opening the >> window for ceph_con_close() to sneak in, close the connection and >> release con->sock. When try_write() is called on the assumption that >> con->state is still valid (i.e. not STANDBY or CLOSED), a NULL sock >> gets passed to the networking stack: >> >> BUG: unable to handle kernel NULL pointer dereference at 0000000000000020 >> IP: selinux_socket_sendmsg+0x5/0x20 >> >> Make sure con->state is valid at the top of try_write() and add an >> explicit BUG_ON for this, similar to try_read(). >> >> Cc: stable@vger.kernel.org >> Link: https://tracker.ceph.com/issues/23706 >> Signed-off-by: Ilya Dryomov >> --- >> net/ceph/messenger.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c >> index fcb40c12b1f8..a7bfc07d2876 100644 >> --- a/net/ceph/messenger.c >> +++ b/net/ceph/messenger.c >> @@ -2569,6 +2569,10 @@ static int try_write(struct ceph_connection *con) >> int ret = 1; >> >> dout("try_write start %p state %lu\n", con, con->state); >> + if (con->state != CON_STATE_PREOPEN && >> + con->state != CON_STATE_NEGOTIATING && >> + con->state != CON_STATE_OPEN) >> + return 0; >> >> more: >> dout("try_write out_kvec_bytes %d\n", con->out_kvec_bytes); >> @@ -2594,6 +2598,8 @@ static int try_write(struct ceph_connection *con) >> } >> >> more_kvec: >> + BUG_ON(!con->sock); >> + >> /* kvec data queued? */ >> if (con->out_kvec_left) { >> ret = write_partial_kvec(con); >> -- >> 2.4.3 >> >> -- >> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html > > Reviewed-by: Jason Dillaman Actually, even though try_write() is where PREOPEN -> CONNECTING transition happens, it is technically possible for try_write() to be called in CONNECTING. Denying it shouldn't break anything, but no point in being overly restrictive here. I have updated the patch: Thanks, Ilya --- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c index a7bfc07d2876..3b3d33ea9ed8 100644 --- a/net/ceph/messenger.c +++ b/net/ceph/messenger.c @@ -2570,6 +2570,7 @@ static int try_write(struct ceph_connection *con) dout("try_write start %p state %lu\n", con, con->state); if (con->state != CON_STATE_PREOPEN && + con->state != CON_STATE_CONNECTING && con->state != CON_STATE_NEGOTIATING && con->state != CON_STATE_OPEN) return 0;