Received: by 2002:ac0:a5a7:0:0:0:0:0 with SMTP id m36-v6csp2026311imm; Tue, 10 Jul 2018 11:51:36 -0700 (PDT) X-Google-Smtp-Source: AAOMgpdLumNyZ9rWogZqhjSJrG6dLPzsGMij2kv7J5FWFdU4Z7opaifPJXXwvCIj1U7g3Z4QhdjU X-Received: by 2002:a63:4506:: with SMTP id s6-v6mr24436663pga.422.1531248696069; Tue, 10 Jul 2018 11:51:36 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1531248696; cv=none; d=google.com; s=arc-20160816; b=mkw3ddg071M9YO55NtBsOYNVrSFIqqcsXcuxdtQ3qz7Kntt4mYyxfDiZpMkCtmeRl5 KgdsijYGI+9JvxEe5yLwXcxeVRzk4ZxgJ4ACbSCF7LTM8TKHGm0wwIchdoVJKfjV2JWH QZMxTNJZBwXRe7TaVDmctBRR7XDR11yd5f2ATfzYQfxdoHqyVrYbwe859dOlDtHCPbc4 AfGKajR//uZazDSOdPvWDO+kC38nOHkfUSYZUZZwmcv1aQHekWSfTbmhgAXA7c28F83L kHrrgZSPZCbDmnOTyP44MsiHCPD9Adrv5KVGCaUouG9kKeyDyOVl6/HC9ETzzXwbyh+s cKXg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:mime-version:user-agent:references :in-reply-to:message-id:date:subject:cc:to:from :arc-authentication-results; bh=hbR6OUf8SBXtpv3t/HEjaZn2dNckwIa6HB+fyiP/cMI=; b=QKUv5F3gRHO0pbhthwTYEMYlvM2mxFA7hz9wli8aOBnEyHAWxbEYaE14zKAW4lrClH zPTnifLjLb/W+9799/0+noUC6+lAA7O0D3+93PBxOND+7K5xg+K8MyBbdCyEAJlwzC1z 5AYEmYYNtR8wRqigL59Y3vO5SG16y3WMYU2M1C1m9IP3UGABIedXnHI3fxwe6/UTRsVL m0JUTe/setBolsmJoUQPII0OQvqC2P78SfktBJOO9FbaGZKG62m3z9D7vIVB6REiSice 3f5PgvPvizB5WCR9T/OuNRvS8rswBGDe2DohCj/3dv6nXbu2neBqrRleIWCNL84Ilm7J gVJw== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id t1-v6si8138300pfb.208.2018.07.10.11.51.21; Tue, 10 Jul 2018 11:51:36 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2388153AbeGJSan (ORCPT + 99 others); Tue, 10 Jul 2018 14:30:43 -0400 Received: from mail.linuxfoundation.org ([140.211.169.12]:45624 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1733214AbeGJSam (ORCPT ); Tue, 10 Jul 2018 14:30:42 -0400 Received: from localhost (LFbn-1-12247-202.w90-92.abo.wanadoo.fr [90.92.61.202]) by mail.linuxfoundation.org (Postfix) with ESMTPSA id 98564EB4; Tue, 10 Jul 2018 18:30:30 +0000 (UTC) From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , stable@vger.kernel.org, Tetsuo Handa , syzbot , Peter Hurley Subject: [PATCH 4.9 06/52] n_tty: Access echo_* variables carefully. Date: Tue, 10 Jul 2018 20:24:34 +0200 Message-Id: <20180710182449.721486951@linuxfoundation.org> X-Mailer: git-send-email 2.18.0 In-Reply-To: <20180710182449.285532226@linuxfoundation.org> References: <20180710182449.285532226@linuxfoundation.org> User-Agent: quilt/0.65 X-stable: review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 4.9-stable review patch. If anyone has any objections, please let me know. ------------------ From: Tetsuo Handa commit ebec3f8f5271139df618ebdf8427e24ba102ba94 upstream. syzbot is reporting stalls at __process_echoes() [1]. This is because since ldata->echo_commit < ldata->echo_tail becomes true for some reason, the discard loop is serving as almost infinite loop. This patch tries to avoid falling into ldata->echo_commit < ldata->echo_tail situation by making access to echo_* variables more carefully. Since reset_buffer_flags() is called without output_lock held, it should not touch echo_* variables. And omit a call to reset_buffer_flags() from n_tty_open() by using vzalloc(). Since add_echo_byte() is called without output_lock held, it needs memory barrier between storing into echo_buf[] and incrementing echo_head counter. echo_buf() needs corresponding memory barrier before reading echo_buf[]. Lack of handling the possibility of not-yet-stored multi-byte operation might be the reason of falling into ldata->echo_commit < ldata->echo_tail situation, for if I do WARN_ON(ldata->echo_commit == tail + 1) prior to echo_buf(ldata, tail + 1), the WARN_ON() fires. Also, explicitly masking with buffer for the former "while" loop, and use ldata->echo_commit > tail for the latter "while" loop. [1] https://syzkaller.appspot.com/bug?id=17f23b094cd80df750e5b0f8982c521ee6bcbf40 Signed-off-by: Tetsuo Handa Reported-by: syzbot Cc: Peter Hurley Cc: stable Signed-off-by: Greg Kroah-Hartman --- drivers/tty/n_tty.c | 42 ++++++++++++++++++++++++------------------ 1 file changed, 24 insertions(+), 18 deletions(-) --- a/drivers/tty/n_tty.c +++ b/drivers/tty/n_tty.c @@ -145,6 +145,7 @@ static inline unsigned char *read_buf_ad static inline unsigned char echo_buf(struct n_tty_data *ldata, size_t i) { + smp_rmb(); /* Matches smp_wmb() in add_echo_byte(). */ return ldata->echo_buf[i & (N_TTY_BUF_SIZE - 1)]; } @@ -320,9 +321,7 @@ static inline void put_tty_queue(unsigne static void reset_buffer_flags(struct n_tty_data *ldata) { ldata->read_head = ldata->canon_head = ldata->read_tail = 0; - ldata->echo_head = ldata->echo_tail = ldata->echo_commit = 0; ldata->commit_head = 0; - ldata->echo_mark = 0; ldata->line_start = 0; ldata->erasing = 0; @@ -621,13 +620,20 @@ static size_t __process_echoes(struct tt old_space = space = tty_write_room(tty); tail = ldata->echo_tail; - while (ldata->echo_commit != tail) { + while (MASK(ldata->echo_commit) != MASK(tail)) { c = echo_buf(ldata, tail); if (c == ECHO_OP_START) { unsigned char op; int no_space_left = 0; /* + * Since add_echo_byte() is called without holding + * output_lock, we might see only portion of multi-byte + * operation. + */ + if (MASK(ldata->echo_commit) == MASK(tail + 1)) + goto not_yet_stored; + /* * If the buffer byte is the start of a multi-byte * operation, get the next byte, which is either the * op code or a control character value. @@ -638,6 +644,8 @@ static size_t __process_echoes(struct tt unsigned int num_chars, num_bs; case ECHO_OP_ERASE_TAB: + if (MASK(ldata->echo_commit) == MASK(tail + 2)) + goto not_yet_stored; num_chars = echo_buf(ldata, tail + 2); /* @@ -732,7 +740,8 @@ static size_t __process_echoes(struct tt /* If the echo buffer is nearly full (so that the possibility exists * of echo overrun before the next commit), then discard enough * data at the tail to prevent a subsequent overrun */ - while (ldata->echo_commit - tail >= ECHO_DISCARD_WATERMARK) { + while (ldata->echo_commit > tail && + ldata->echo_commit - tail >= ECHO_DISCARD_WATERMARK) { if (echo_buf(ldata, tail) == ECHO_OP_START) { if (echo_buf(ldata, tail + 1) == ECHO_OP_ERASE_TAB) tail += 3; @@ -742,6 +751,7 @@ static size_t __process_echoes(struct tt tail++; } + not_yet_stored: ldata->echo_tail = tail; return old_space - space; } @@ -752,6 +762,7 @@ static void commit_echoes(struct tty_str size_t nr, old, echoed; size_t head; + mutex_lock(&ldata->output_lock); head = ldata->echo_head; ldata->echo_mark = head; old = ldata->echo_commit - ldata->echo_tail; @@ -760,10 +771,12 @@ static void commit_echoes(struct tty_str * is over the threshold (and try again each time another * block is accumulated) */ nr = head - ldata->echo_tail; - if (nr < ECHO_COMMIT_WATERMARK || (nr % ECHO_BLOCK > old % ECHO_BLOCK)) + if (nr < ECHO_COMMIT_WATERMARK || + (nr % ECHO_BLOCK > old % ECHO_BLOCK)) { + mutex_unlock(&ldata->output_lock); return; + } - mutex_lock(&ldata->output_lock); ldata->echo_commit = head; echoed = __process_echoes(tty); mutex_unlock(&ldata->output_lock); @@ -814,7 +827,9 @@ static void flush_echoes(struct tty_stru static inline void add_echo_byte(unsigned char c, struct n_tty_data *ldata) { - *echo_buf_addr(ldata, ldata->echo_head++) = c; + *echo_buf_addr(ldata, ldata->echo_head) = c; + smp_wmb(); /* Matches smp_rmb() in echo_buf(). */ + ldata->echo_head++; } /** @@ -1883,30 +1898,21 @@ static int n_tty_open(struct tty_struct struct n_tty_data *ldata; /* Currently a malloc failure here can panic */ - ldata = vmalloc(sizeof(*ldata)); + ldata = vzalloc(sizeof(*ldata)); if (!ldata) - goto err; + return -ENOMEM; ldata->overrun_time = jiffies; mutex_init(&ldata->atomic_read_lock); mutex_init(&ldata->output_lock); tty->disc_data = ldata; - reset_buffer_flags(tty->disc_data); - ldata->column = 0; - ldata->canon_column = 0; - ldata->num_overrun = 0; - ldata->no_room = 0; - ldata->lnext = 0; tty->closing = 0; /* indicate buffer work may resume */ clear_bit(TTY_LDISC_HALTED, &tty->flags); n_tty_set_termios(tty, NULL); tty_unthrottle(tty); - return 0; -err: - return -ENOMEM; } static inline int input_available_p(struct tty_struct *tty, int poll)