Received: by 2002:a05:6a10:22f:0:0:0:0 with SMTP id 15csp1123284pxk; Fri, 4 Sep 2020 00:45:15 -0700 (PDT) X-Google-Smtp-Source: ABdhPJxXlehw0XmvfwZP1aAIBEI8ZRKnM6suVfZjZluUqEly7xm+Dz62y5t98u2Xk+V2KqOdUief X-Received: by 2002:a17:906:829a:: with SMTP id h26mr6385703ejx.11.1599205515428; Fri, 04 Sep 2020 00:45:15 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1599205515; cv=none; d=google.com; s=arc-20160816; b=bCN+q1cUi3hL1jhv2PZjwZK2rh99mRC+qRdygmA2TDUAqowXW8HqKWb5/E0Gd82dLe a+NH8mC/Xo3z3kmZXVL+3Vt4s0rJBVnYqPWE4JP8qN4eQiD+hLLu5EB9H2/8C33LW9wa WeLwf1RxqrNfmUWtD7AjDzVAQY4AiXZERkvVSLF/qejPVYMlEsCKat4/OfROzXGXDWnQ g6i3Jzh0K/aodfw9gdxRh3Sa76YD8Q5Mvn39uwQycpCKTS7pag/aZe0NrCHC85OOs9mN P6lHssTqwU9eZMnb9RpZcd1Qcrd0x5McyorFZ9gWT7ZNZVY6ifh8noztu5Dq/zhL8z+1 6uFQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:in-reply-to:content-disposition :mime-version:references:message-id:subject:cc:to:from:date :dkim-signature; bh=tQaAklUi4mOHolVXLrurT6V8jWl51s1fx1oUnBoO5Jw=; b=eEGZochf+ZW2tMhTENr7mCRMSfP2sqinLmLTFyCAbOlR6Jm5PQGbsrOUYyNgC8RjIV 2zch37jfJzkYfuS6Uc5AMEE7pdwm9dmUgveCQTgjpVvalqtLX9Cc1F2F/W8HW2YODv97 8MboTUTMtBR3A7Anw59YapqalxcG3E6zM33+pOR2lEK0EgLG5B8snyE542ulhx4DZnNA nrGxbv5zZB5B6nPuuDd0Ctrcyc7oVW3Akjrc/OMmtxOoU8ZchKZ10SllBI/pFbAd4pnJ 012dCvg0yeBDHs6xkWkHIdxs36mDDlOr8yNAKCoHFBnX6SCTgT02BbW4ryf+K15zSDG6 PS2A== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=bNMhJyTs; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id q25si3212938edr.402.2020.09.04.00.44.52; Fri, 04 Sep 2020 00:45:15 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=bNMhJyTs; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726425AbgIDHni (ORCPT + 99 others); Fri, 4 Sep 2020 03:43:38 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53916 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726089AbgIDHnh (ORCPT ); Fri, 4 Sep 2020 03:43:37 -0400 Received: from mail-pg1-x543.google.com (mail-pg1-x543.google.com [IPv6:2607:f8b0:4864:20::543]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 69B5EC061244 for ; Fri, 4 Sep 2020 00:43:37 -0700 (PDT) Received: by mail-pg1-x543.google.com with SMTP id g29so3919310pgl.2 for ; Fri, 04 Sep 2020 00:43:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=tQaAklUi4mOHolVXLrurT6V8jWl51s1fx1oUnBoO5Jw=; b=bNMhJyTsoVn1gKktUBUxgiXMXLoUoVG4WJERz+LrnRQFWwPg3JjS0tDQIlZJR+RSjF Roo8emi2j4ZW2gCmGswCZGlg7atyt9XdJD/n68RuwLxnIeMBUkPhFvsd32ye7MXFdeUD STKGEIxy6HzCwU1xvNkntsaqMoSP9ZToLTi/zPMI6sRgNp7xJnMMUiG2iZvlJFSifiV3 KVUR7DufJU48KbT+p+tnQxPK37ia9MdBudy5LoWlj6dheB4xbwVNvzSJS4GvI8UjrcwA T8dFV3nBdz1l4TYPvDn9u4qWdFIcKIV8gqgx7RgnD8xEl06D9orcNbA5X62k5lnZjU5N pNHg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=tQaAklUi4mOHolVXLrurT6V8jWl51s1fx1oUnBoO5Jw=; b=AG1zh+w7wdRUdgiwmzp589Hwvjsq2qBV4lr2kME2UeyqPne6Iiyr3/5hP1Y4g9vmKg v7plrOxAo1gG9RYRq+4TV7MnUWqduNltkulJWdEVdomu5vtLiHT3aM+lWmbcl/p18G8O 7WjpUC70K3qS4FCZfHAo/6H+rckjX18ZLj/OJ0LxTNIvDTEOUjqkMUnVxTE6/Lc2J8S7 IqUagpRHb/bndJmN9hIX1/skvBcdNsaZv5J0DT/NV3B1AqWx3t952T2+wuXeaTEvTWMg D5CtsULm2cH4T7fFA9wDAJ3dPcgY/5vhBA5pmMLKx/UHTgBG48oa7ectq0ecLcUA9bdI Lwow== X-Gm-Message-State: AOAM533u6htOFdR5muHJI39iDHFUoFBSIml6cqJQnIyUi8G8BpwR10pX FAFapMpz0WhB9RrHgwmsuKM= X-Received: by 2002:a63:ca0c:: with SMTP id n12mr3215793pgi.209.1599205416920; Fri, 04 Sep 2020 00:43:36 -0700 (PDT) Received: from localhost ([2409:10:2e40:5100:6e29:95ff:fe2d:8f34]) by smtp.gmail.com with ESMTPSA id b20sm5966248pfb.198.2020.09.04.00.43.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 04 Sep 2020 00:43:35 -0700 (PDT) Date: Fri, 4 Sep 2020 16:43:33 +0900 From: Sergey Senozhatsky To: Artem Savkov Cc: gregkh@linuxfoundation.org, jirislaby@kernel.org, threeearcat@gmail.com, sergey.senozhatsky@gmail.com, linux-kernel@vger.kernel.org Subject: Re: [PATCH] pty: do tty_flip_buffer_push without port->lock in pty_write Message-ID: <20200904074333.GA410@jagdpanzerIV.localdomain> References: <20200901120157.3412245-1-asavkov@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200901120157.3412245-1-asavkov@redhat.com> Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On (20/09/01 14:01), Artem Savkov wrote: [..] > It looks like the commit was aimed to protect tty_insert_flip_string and > there is no need for tty_flip_buffer_push to be under this lock. > [..] > @@ -120,10 +120,10 @@ static int pty_write(struct tty_struct *tty, const unsigned char *buf, int c) > 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); > - spin_unlock_irqrestore(&to->port->lock, flags); Performing unprotected smp_store_release(&buf->tail->commit, buf->tail->used); does not look safe to me. This path can be called concurrently - "pty_write vs console's IRQ handler (TX/RX)", for instance. Doing this queue_work(system_unbound_wq, &buf->work); outside of port->lock scope also sounds like possible concurrent data modification. I'm not sure I see how this patch is safe. -ss