From patchwork Fri Mar 17 11:33:17 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Ilpo_J=C3=A4rvinen?= X-Patchwork-Id: 664696 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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 15420C7618B for ; Fri, 17 Mar 2023 11:37:29 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230256AbjCQLh1 (ORCPT ); Fri, 17 Mar 2023 07:37:27 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55692 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230266AbjCQLhO (ORCPT ); Fri, 17 Mar 2023 07:37:14 -0400 Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 98C8FAF6A7; Fri, 17 Mar 2023 04:36:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1679053009; x=1710589009; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=sSrP2H0F2QdS2sJv68twXcd58hArlvzYGfTg+emznYc=; b=Eau4txrx7eiqi2m8EWUdpnK4VNrKBaqDBy05KXhGusH3Tp2vA3kePW9Y 8VVHHr1MZt/v7WhLRYSCoEpVbcLf4S/JjokXlvnmANuFDq3/YCAQiE1Fj tWqYiFH8j3MnSzaIUbaa4xs+nrbtEQElbMXxCDhni/lE2lGaTp2YXssYD sCa3XjLrLuk1U9yowgT3P3lD9pG25yQ9P/ptcc9FY51jKjKOfs0Y4bSKQ QZqaPjBQ5B5JMf43jezJ6Q2D/No+dpP+Lj5YrFFBjZAr9NetnktBXpPQs 7tSbXCASj0HB+k8n/6ChcmuDNaqF3nqLglFbVi0NL3eD7RWIYsQdKTMHo A==; X-IronPort-AV: E=McAfee;i="6600,9927,10651"; a="339779168" X-IronPort-AV: E=Sophos;i="5.98,268,1673942400"; d="scan'208";a="339779168" Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 17 Mar 2023 04:33:33 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10651"; a="680270115" X-IronPort-AV: E=Sophos;i="5.98,268,1673942400"; d="scan'208";a="680270115" Received: from bstach-mobl1.ger.corp.intel.com (HELO ijarvine-MOBL2.ger.corp.intel.com) ([10.251.221.222]) by orsmga002-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 17 Mar 2023 04:33:31 -0700 From: =?utf-8?q?Ilpo_J=C3=A4rvinen?= To: linux-serial@vger.kernel.org, Greg Kroah-Hartman , Jiri Slaby , David Laight , linux-kernel@vger.kernel.org Cc: =?utf-8?q?Ilpo_J=C3=A4rvinen?= , stable@vger.kernel.org Subject: [PATCH v2 1/2] tty: Prevent writing chars during tcsetattr TCSADRAIN/FLUSH Date: Fri, 17 Mar 2023 13:33:17 +0200 Message-Id: <20230317113318.31327-2-ilpo.jarvinen@linux.intel.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20230317113318.31327-1-ilpo.jarvinen@linux.intel.com> References: <20230317113318.31327-1-ilpo.jarvinen@linux.intel.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-serial@vger.kernel.org If userspace races tcsetattr() with a write, the drained condition might not be guaranteed by the kernel. There is a race window after checking Tx is empty before tty_set_termios() takes termios_rwsem for write. During that race window, more characters can be queued by a racing writer. Any ongoing transmission might produce garbage during HW's ->set_termios() call. The intent of TCSADRAIN/FLUSH seems to be preventing such a character corruption. If those flags are set, take tty's write lock to stop any writer before performing the lower layer Tx empty check and wait for the pending characters to be sent (if any). The initial wait for all-writers-done must be placed outside of tty's write lock to avoid deadlock which makes it impossible to use tty_wait_until_sent(). The write lock is retried if a racing write is detected. Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Cc: stable@vger.kernel.org Signed-off-by: Ilpo Järvinen --- drivers/tty/tty.h | 2 ++ drivers/tty/tty_io.c | 4 ++-- drivers/tty/tty_ioctl.c | 45 ++++++++++++++++++++++++++++++----------- 3 files changed, 37 insertions(+), 14 deletions(-) diff --git a/drivers/tty/tty.h b/drivers/tty/tty.h index f45cd683c02e..1e0d80e98d26 100644 --- a/drivers/tty/tty.h +++ b/drivers/tty/tty.h @@ -62,6 +62,8 @@ int __tty_check_change(struct tty_struct *tty, int sig); int tty_check_change(struct tty_struct *tty); void __stop_tty(struct tty_struct *tty); void __start_tty(struct tty_struct *tty); +void tty_write_unlock(struct tty_struct *tty); +int tty_write_lock(struct tty_struct *tty, int ndelay); void tty_vhangup_session(struct tty_struct *tty); void tty_open_proc_set_tty(struct file *filp, struct tty_struct *tty); int tty_signal_session_leader(struct tty_struct *tty, int exit_session); diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c index 766750e355ac..cfb3da0dee47 100644 --- a/drivers/tty/tty_io.c +++ b/drivers/tty/tty_io.c @@ -933,13 +933,13 @@ static ssize_t tty_read(struct kiocb *iocb, struct iov_iter *to) return i; } -static void tty_write_unlock(struct tty_struct *tty) +void tty_write_unlock(struct tty_struct *tty) { mutex_unlock(&tty->atomic_write_lock); wake_up_interruptible_poll(&tty->write_wait, EPOLLOUT); } -static int tty_write_lock(struct tty_struct *tty, int ndelay) +int tty_write_lock(struct tty_struct *tty, int ndelay) { if (!mutex_trylock(&tty->atomic_write_lock)) { if (ndelay) diff --git a/drivers/tty/tty_ioctl.c b/drivers/tty/tty_ioctl.c index 12983ce4e43e..a13e3797c477 100644 --- a/drivers/tty/tty_ioctl.c +++ b/drivers/tty/tty_ioctl.c @@ -500,21 +500,42 @@ static int set_termios(struct tty_struct *tty, void __user *arg, int opt) tmp_termios.c_ispeed = tty_termios_input_baud_rate(&tmp_termios); tmp_termios.c_ospeed = tty_termios_baud_rate(&tmp_termios); - ld = tty_ldisc_ref(tty); + if (opt & (TERMIOS_FLUSH|TERMIOS_WAIT)) { +retry_write_wait: + retval = wait_event_interruptible(tty->write_wait, !tty_chars_in_buffer(tty)); + if (retval < 0) + return retval; - if (ld != NULL) { - if ((opt & TERMIOS_FLUSH) && ld->ops->flush_buffer) - ld->ops->flush_buffer(tty); - tty_ldisc_deref(ld); - } + if (tty_write_lock(tty, 0) < 0) + goto retry_write_wait; - if (opt & TERMIOS_WAIT) { - tty_wait_until_sent(tty, 0); - if (signal_pending(current)) - return -ERESTARTSYS; - } + /* Racing writer? */ + if (tty_chars_in_buffer(tty)) { + tty_write_unlock(tty); + goto retry_write_wait; + } - tty_set_termios(tty, &tmp_termios); + ld = tty_ldisc_ref(tty); + if (ld != NULL) { + if ((opt & TERMIOS_FLUSH) && ld->ops->flush_buffer) + ld->ops->flush_buffer(tty); + tty_ldisc_deref(ld); + } + + if ((opt & TERMIOS_WAIT) && tty->ops->wait_until_sent) { + tty->ops->wait_until_sent(tty, 0); + if (signal_pending(current)) { + tty_write_unlock(tty); + return -ERESTARTSYS; + } + } + + tty_set_termios(tty, &tmp_termios); + + tty_write_unlock(tty); + } else { + tty_set_termios(tty, &tmp_termios); + } /* FIXME: Arguably if tmp_termios == tty->termios AND the actual requested termios was not tmp_termios then we may