From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id BF97DC43334 for ; Thu, 2 Jun 2022 04:48:34 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id F0A006B0071; Thu, 2 Jun 2022 00:48:33 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id EB6376B0072; Thu, 2 Jun 2022 00:48:33 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D7EEF6B0073; Thu, 2 Jun 2022 00:48:33 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id C57CE6B0071 for ; Thu, 2 Jun 2022 00:48:33 -0400 (EDT) Received: from smtpin27.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 3664D346F9 for ; Thu, 2 Jun 2022 04:48:33 +0000 (UTC) X-FDA: 79532064906.27.C01E519 Received: from mail-wr1-f44.google.com (mail-wr1-f44.google.com [209.85.221.44]) by imf09.hostedemail.com (Postfix) with ESMTP id 873B5140088 for ; Thu, 2 Jun 2022 04:48:17 +0000 (UTC) Received: by mail-wr1-f44.google.com with SMTP id q21so4881032wra.2 for ; Wed, 01 Jun 2022 21:48:32 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:cc:references:from:in-reply-to :content-transfer-encoding; bh=AUkIrxS+XdWPb8Cp2nIkEd8cc/NScUdoM6o1x+2YUu4=; b=wtQoc6BQVELALLrGzj7R48zkb8bc8Xzj/jutxi6KuB8geMmNrOQ3th/BkkWIU5AnwE bWRt0ki3VCVU6tCSLRCRsHjFfLYduLLkbMYzVDLgQmUCOj9adhnIoP388amn72HOamfZ HMNJ/sYIFvhs3IFAPF6xSZN5zSj74WSZ1t6VhMKxQul8lp5UbC3/OqQ6kfgv3hKUk5XC VMCQTpec0xO4fEbbBeJrANcyBZA4OoJhRFf7nVLAQvloKYeot9Tws4RtXucvluYT39wm Fypf7ZAooe2y4VX3qO0XiM7FCPhbAf5k4na08nfX5d2pkuBxoEzyBxKa8Vyy7VBNVOdn m/uA== X-Gm-Message-State: AOAM530Wt+uTenZOdufKTcdrTSgluVCR4/dwH7ysaRfF340XnFb1GkzA LYHTxnh4/G6Ee5LsKHIMeDQ= X-Google-Smtp-Source: ABdhPJyIz/NVsaPw8UNBIeCQcB01t4PENq2t2NN6Gs2dK1ZUgPkErt1toDQjfelA+6K+mFXYkh2bTw== X-Received: by 2002:a5d:40c7:0:b0:210:3135:e662 with SMTP id b7-20020a5d40c7000000b002103135e662mr1954347wrq.280.1654145311366; Wed, 01 Jun 2022 21:48:31 -0700 (PDT) Received: from ?IPV6:2a0b:e7c0:0:107::49? ([2a0b:e7c0:0:107::49]) by smtp.gmail.com with ESMTPSA id r184-20020a1c2bc1000000b0039768532c79sm4083788wmr.6.2022.06.01.21.48.30 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 01 Jun 2022 21:48:30 -0700 (PDT) Message-ID: <0dc35f2e-746c-bcec-160c-645055a6f8d2@kernel.org> Date: Thu, 2 Jun 2022 06:48:29 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.9.0 Subject: Re: CVE-2022-1462: race condition vulnerability in drivers/tty/tty_buffers.c Content-Language: en-US To: Hillf Danton , Dan Carpenter Cc: ChenBigNB , Greg Kroah-Hartman , linux-mm@kvack.org, linux-kernel@vger.kernel.org References: <20220602024857.4808-1-hdanton@sina.com> From: Jiri Slaby In-Reply-To: <20220602024857.4808-1-hdanton@sina.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Authentication-Results: imf09.hostedemail.com; dkim=none; spf=pass (imf09.hostedemail.com: domain of jirislaby@gmail.com designates 209.85.221.44 as permitted sender) smtp.mailfrom=jirislaby@gmail.com; dmarc=fail reason="SPF not aligned (relaxed), No valid DKIM" header.from=kernel.org (policy=none) X-Rspamd-Server: rspam03 X-Rspamd-Queue-Id: 873B5140088 X-Rspam-User: X-Stat-Signature: 1hg3pcue1hcfgfefh55tq3ojs36pgs8q X-HE-Tag: 1654145297-554614 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On 02. 06. 22, 4:48, Hillf Danton wrote: > On Wed, 1 Jun 2022 21:34:26 +0300 Dan Carpenter wrote: >> Hi Greg, Jiri, >> >> I searched lore.kernel.org and it seemed like CVE-2022-1462 might not >> have ever been reported to you? Here is the original email with the >> syzkaller reproducer. >> >> https://seclists.org/oss-sec/2022/q2/155 >> >> The reporter proposed a fix, but it won't work. Smatch says that some >> of the callers are already holding the port->lock. For example, >> sci_dma_rx_complete() will deadlock. > > Hi Dan > > To erase the deadlock above, we need to add another helper folding > tty_insert_flip_string() and tty_flip_buffer_push() into one nutshell, > with buf->tail covered by port->lock. > > The diff attached in effect reverts > 71a174b39f10 ("pty: do tty_flip_buffer_push without port->lock in pty_write"). > > Only for thoughts now. I think this the likely the best approach. Except few points inlined below. Another would be to split tty_flip_buffer_push() into two and call only the first one (doing smp_store_release()) inside the lock. I tried that already, but it looks much worse. Another would be to add flags to tty_flip_buffer_push(). Like ONLY_ADVANCE and ONLY_QUEUE. Call with the first under the lock, the second outside. Ideas, comments? > Hillf > > +++ b/drivers/tty/pty.c > @@ -116,15 +116,8 @@ static int pty_write(struct tty_struct * > if (tty->flow.stopped) > return 0; > > - if (c > 0) { > - spin_lock_irqsave(&to->port->lock, flags); > - /* Stuff the data into the input queue of the other end */ > - c = tty_insert_flip_string(to->port, buf, c); > - spin_unlock_irqrestore(&to->port->lock, flags); > - /* And shovel */ > - if (c) > - tty_flip_buffer_push(to->port); > - } > + if (c > 0) > + c = tty_flip_insert_and_push_buffer(to->port, buf, c); > return c; > } > > +++ b/drivers/tty/tty_buffer.c > @@ -554,6 +554,26 @@ void tty_flip_buffer_push(struct tty_por > } > EXPORT_SYMBOL(tty_flip_buffer_push); > > +int tty_flip_insert_and_push_buffer(struct tty_port *port, const unsigned char *string, int cnt) It should be _insert_string_, IMO. > +{ > + struct tty_bufhead *buf = &port->buf; > + unsigned long flags; > + > + spin_lock_irqsave(&port->lock, flags); > + cnt = tty_insert_flip_string(port, string, cnt); > + if (cnt) { > + /* > + * Paired w/ acquire in flush_to_ldisc(); ensures flush_to_ldisc() sees > + * buffer data. > + */ > + smp_store_release(&buf->tail->commit, buf->tail->used); > + } > + spin_unlock_irqrestore(&port->lock, flags); > + queue_work(system_unbound_wq, &buf->work); \n here please. > + return cnt; > +} > +EXPORT_SYMBOL(tty_flip_insert_and_push_buffer); No need to export this, right? thanks, -- js suse labs