From patchwork Tue Aug 29 00:12:00 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jordan Niethe X-Patchwork-Id: 1827043 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20221208 header.b=R3gmJE1t; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=lists.ozlabs.org (client-ip=112.213.38.117; helo=lists.ozlabs.org; envelope-from=slof-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org; receiver=patchwork.ozlabs.org) Received: from lists.ozlabs.org (lists.ozlabs.org [112.213.38.117]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1)) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4RZSY045bMz1yfy for ; Tue, 29 Aug 2023 10:12:19 +1000 (AEST) Authentication-Results: lists.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20221208 header.b=R3gmJE1t; dkim-atps=neutral Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4RZSXz0Spyz2ytZ for ; Tue, 29 Aug 2023 10:12:19 +1000 (AEST) X-Original-To: slof@lists.ozlabs.org Delivered-To: slof@lists.ozlabs.org Authentication-Results: lists.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20221208 header.b=R3gmJE1t; dkim-atps=neutral Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gmail.com (client-ip=2607:f8b0:4864:20::635; helo=mail-pl1-x635.google.com; envelope-from=jniethe5@gmail.com; receiver=lists.ozlabs.org) Received: from mail-pl1-x635.google.com (mail-pl1-x635.google.com [IPv6:2607:f8b0:4864:20::635]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4RZSXr1566z2xjw for ; Tue, 29 Aug 2023 10:12:10 +1000 (AEST) Received: by mail-pl1-x635.google.com with SMTP id d9443c01a7336-1c0d5b16aacso20733705ad.1 for ; Mon, 28 Aug 2023 17:12:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1693267928; x=1693872728; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=lDv8c+XWAg1F3cIJFIQcYbWLzUD5ekuBcEW7hsJ1UXQ=; b=R3gmJE1thE8xuX4t2Ixx1I3398fJqTr/ldkuT5dY1weUbPUwfEL5fVFodJ7FD/6zR2 EKmgl8Ys/2nAlAE8RWpIqps9lRVIeYJV7gR31wICekQ5uwQ2MiKZT9EoiJhVHC0BomrG BBa8gA//umXgy9m9kPbZRzcHxAlJBRSXin7M61Ru2qqD6ahar9qVTik5KGV5o1S0rhFn OvkRq9ewysEAY1EN5jQk7xPdr+t/UOG+0tT7OUEjzL5UnQDXKc3xqOkIiAUnr+LbGhVu p88CY/Os6z745cgZ1hUN7GJ6RZI+/rO/CfscKmKbAho5vsvJdb4X+iZjnK/AQqjwiLqh DR7g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1693267928; x=1693872728; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=lDv8c+XWAg1F3cIJFIQcYbWLzUD5ekuBcEW7hsJ1UXQ=; b=cezIilKSivrQ5BEbHi5Hq0eAQWm0IWHKhnWbzg0P2wlh6TLBrP1K3FSmsvslc3ljNH 8jRnD/xiCROj7x5pWmxK/owv7rU+/O6jTgyJadMFKimnlv0AE+5Z8hF1pzGBWZlZNW1S 1ibnb5XVaADOwKpQvnI8KRKyPfB6Zt2Ybio6TsMcFvt9ukx5pFx2yIdh21HORIE3UNoM ubNuZaRl6Zq2+jvWKOkN9s40eAxGW1tE7Sw9F8/mm9Oe2R1eNRYrmRf3nxZzwREfxF98 IswKVoJLZyxUEajHaAq7XMGeaGAVRMqEQUHvLmAsRiJseNsXJ9m3L8C1gpSVrMaH1/kJ bw/A== X-Gm-Message-State: AOJu0YzmhdxGSruAikIbBNpQA9SUdHemu30nkOMkYAvzb31UOKnWttXO +1e9YHzM09eNu9abq30KBZ3Z2wv6utg= X-Google-Smtp-Source: AGHT+IHGqBOzBVNz9s3l7g6kqqoVtgYzvMAtEjXKh5KcWQQzmE1TfqHY4uS2lrICsLjjI4Sh2rclNQ== X-Received: by 2002:a17:902:8c8c:b0:1c1:de2b:e1d0 with SMTP id t12-20020a1709028c8c00b001c1de2be1d0mr3179060plo.32.1693267928144; Mon, 28 Aug 2023 17:12:08 -0700 (PDT) Received: from pwon.ozlabs.ibm.com ([146.112.118.69]) by smtp.gmail.com with ESMTPSA id y3-20020a170902ed4300b001b80760fd04sm7979870plb.112.2023.08.28.17.12.05 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 28 Aug 2023 17:12:07 -0700 (PDT) From: Jordan Niethe To: slof@lists.ozlabs.org Date: Tue, 29 Aug 2023 10:12:00 +1000 Message-Id: <20230829001201.11892-1-jniethe5@gmail.com> X-Mailer: git-send-email 2.39.3 MIME-Version: 1.0 Subject: [SLOF] [PATCH v2 1/2] virtio-serial: Make read and write methods report failure X-BeenThere: slof@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "Patches for https://github.com/aik/SLOF" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: thuth@redhat.com, kconsul@linux.vnet.ibm.com, groug@kaod.org Errors-To: slof-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org Sender: "SLOF" From: Kautuk Consul The read and write methods return successfully even if the virtio device is closed (virtiodev is 0) and it is not able to send or receive any characters. Make the read and write methods return 0 to indicate they did not succeed in this case. This also fixes an invalid stack access in the read method. Fixes: 8174acd ("virtio-serial: Close device completely") Signed-off-by: Kautuk Consul Signed-off-by: Jordan Niethe Reviewed-by: Thomas Huth --- v2: - Rework commit message slightly --- board-qemu/slof/virtio-serial.fs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/board-qemu/slof/virtio-serial.fs b/board-qemu/slof/virtio-serial.fs index 82868e2..41e2e04 100644 --- a/board-qemu/slof/virtio-serial.fs +++ b/board-qemu/slof/virtio-serial.fs @@ -67,7 +67,7 @@ virtiodev virtio-serial-init drop ; : write ( addr len -- actual ) - virtiodev 0= IF nip EXIT THEN + virtiodev 0= IF 2drop 0 EXIT THEN tuck 0 ?DO dup c@ virtiodev SWAP virtio-serial-putchar @@ -78,7 +78,7 @@ virtiodev virtio-serial-init drop : read ( addr len -- actual ) 0= IF drop 0 EXIT THEN - virtiodev 0= IF nip EXIT THEN + virtiodev 0= IF drop 0 EXIT THEN virtiodev virtio-serial-haschar 0= IF 0 swap c! -2 EXIT THEN virtiodev virtio-serial-getchar swap c! 1 ; From patchwork Tue Aug 29 00:12:01 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jordan Niethe X-Patchwork-Id: 1827044 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20221208 header.b=YC1QfEjx; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=lists.ozlabs.org (client-ip=2404:9400:2:0:216:3eff:fee1:b9f1; helo=lists.ozlabs.org; envelope-from=slof-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org; receiver=patchwork.ozlabs.org) Received: from lists.ozlabs.org (lists.ozlabs.org [IPv6:2404:9400:2:0:216:3eff:fee1:b9f1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4RZSY13y51z1yg3 for ; Tue, 29 Aug 2023 10:12:21 +1000 (AEST) Authentication-Results: lists.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20221208 header.b=YC1QfEjx; dkim-atps=neutral Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4RZSY13B71z3bTj for ; Tue, 29 Aug 2023 10:12:21 +1000 (AEST) X-Original-To: slof@lists.ozlabs.org Delivered-To: slof@lists.ozlabs.org Authentication-Results: lists.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20221208 header.b=YC1QfEjx; dkim-atps=neutral Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gmail.com (client-ip=2607:f8b0:4864:20::62f; helo=mail-pl1-x62f.google.com; envelope-from=jniethe5@gmail.com; receiver=lists.ozlabs.org) Received: from mail-pl1-x62f.google.com (mail-pl1-x62f.google.com [IPv6:2607:f8b0:4864:20::62f]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4RZSXw5YkTz2yKy for ; Tue, 29 Aug 2023 10:12:16 +1000 (AEST) Received: by mail-pl1-x62f.google.com with SMTP id d9443c01a7336-1c0d0bf18d6so30397545ad.0 for ; Mon, 28 Aug 2023 17:12:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1693267931; x=1693872731; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=vSzp/XBij+QyrnuQScDuSmsU9YzX6u5nBntZIAUNt5s=; b=YC1QfEjxGROFs4kqbRBU14BvUpccjUTikNPnbGoUnxNY2VWjUoG1DNG3VzIfs7fpqJ C6ANHg4yiuJ9E6yfgA9TBI5lGasxxN5NxK64Cq4PnSDLeXFYotKEp6lKSfWt4h3zSQ9U tXHHyyWrmPZjND+pJnBPmMVUMjOq8amIUxnkfJuU/PiITgZ6xA1AAMleW8WN75kM8W8i cCQnU1u13S0IlwWA2bOfQsCOPaiAPV55Rkx2ukieMOwMY+9GGOjvEvZDHWr6rLU9cAEB 0p//QcxHvRlezpiT78mWiLj4Xp694xF0C9NCZrNnYPZ4rBbV9QicEQ89wqxyqdoSvmKA 7jCg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1693267931; x=1693872731; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=vSzp/XBij+QyrnuQScDuSmsU9YzX6u5nBntZIAUNt5s=; b=S6mXKY3XepTAJmhUrhrHtSYhff4W48cPPssAVJeJ9xJnAH6vYQBXA/leXB/QEQ1A+q rG0L6LYmInIUcpWsi9wUXer1QJBxaIvCDSYzNpeXkjjm7pyIrxG0nQeWOEIjzjnEfy1q QRXnT90/r1BzQ0jxIZVIctk1BykiRfuqfEFqbWlVky8CfdDHRqHX3iaSvXNh0aBfsLDz BcPSSPo9HS75zIhjkXxO2yIoAVFAh40dWee/iR5qRQn0ZRB4dxHLX+QWOgv5l3gA6nP8 AzTTcmwkXy5xPpMocAbJ/NbsqTlNeR2stoBrFJgzmklwqlcoE4GtZCwBsMx8s1QJcR6+ PFMA== X-Gm-Message-State: AOJu0YwbSxvIkphCjz0lGFQU+rxoc3/rRoM6KHxl8rbUKGg7vBZcT1GE YtNZFBNiwTcOawCSU2m1K33Qhtnvzvs= X-Google-Smtp-Source: AGHT+IG7jwNcw5duv6t0gBjyUep6AEO8eacLmmDBiJZ+px4TU63l1FO81R64/6suPAzZ3fS7mmYMMg== X-Received: by 2002:a17:902:ea04:b0:1ab:11c8:777a with SMTP id s4-20020a170902ea0400b001ab11c8777amr33498499plg.13.1693267931239; Mon, 28 Aug 2023 17:12:11 -0700 (PDT) Received: from pwon.ozlabs.ibm.com ([146.112.118.69]) by smtp.gmail.com with ESMTPSA id y3-20020a170902ed4300b001b80760fd04sm7979870plb.112.2023.08.28.17.12.08 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 28 Aug 2023 17:12:10 -0700 (PDT) From: Jordan Niethe To: slof@lists.ozlabs.org Date: Tue, 29 Aug 2023 10:12:01 +1000 Message-Id: <20230829001201.11892-2-jniethe5@gmail.com> X-Mailer: git-send-email 2.39.3 In-Reply-To: <20230829001201.11892-1-jniethe5@gmail.com> References: <20230829001201.11892-1-jniethe5@gmail.com> MIME-Version: 1.0 Subject: [SLOF] [PATCH v2 2/2] virtio-serial: Do not close stdout on quiesce X-BeenThere: slof@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "Patches for https://github.com/aik/SLOF" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: thuth@redhat.com, kconsul@linux.vnet.ibm.com, groug@kaod.org Errors-To: slof-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org Sender: "SLOF" Commit 76fee95 ("slof: Only close stdout for virtio-serial devices") says that commit cf28264 ("virtio-serial: Rework shutdown sequence") fixed a hang. The problem was believed to be that it was necessary to close stdout to shutdown the underlying virtio device. Commit cf28264 ("virtio-serial: Rework shutdown sequence") closed stdout on quiesce. This meant when prom_init() called write on stdout after quiesce, there is a use after free so this is unreliable, and can also hang (especially after reboots). Quiescing is intended to put hardware into a safe state for the client to take over. It is incorrect for SLOF to close ihandles that the client could still be using, even after a quiesce. Rather than closing the stdout device, all that needs to happen is to ensure virtio-serial-shutdown gets called. On quiesce, close the virtio device, but leave the stdout device itself open. Commit 8174acd ("virtio-serial: Close device completely") handles reads and writes as no-ops if the underlying virtio device is closed so there is no problem with the client calling "write" on stdout after this, but no output will be displayed. Fixes: cf28264 ("virtio-serial: Rework shutdown sequence") Debugged-by: Kautuk Consul Co-developed-by: Kautuk Consul Signed-off-by: Kautuk Consul Signed-off-by: Jordan Niethe Reviewed-by: Thomas Huth --- v2: - Reword init comment --- board-qemu/slof/virtio-serial.fs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/board-qemu/slof/virtio-serial.fs b/board-qemu/slof/virtio-serial.fs index 41e2e04..de42cc7 100644 --- a/board-qemu/slof/virtio-serial.fs +++ b/board-qemu/slof/virtio-serial.fs @@ -33,16 +33,14 @@ virtio-setup-vd VALUE virtiodev : virtio-serial-term-key? virtiodev virtio-serial-haschar ; : virtio-serial-term-key BEGIN virtio-serial-term-key? UNTIL virtiodev virtio-serial-getchar ; -: virtio-serial-close-stdout s" stdout" get-chosen IF decode-int nip nip close-dev THEN ; - \ Basic device initialization - which has only to be done once : init ( -- ) virtiodev virtio-serial-init drop TRUE to initialized? - \ Linux closes stdin at some point in prom_init(). This internally triggers a - \ quiesce in SLOF. We must ensure stdout gets closed as well otherwise the - \ device cannot be reset properly and the boot will hang. - ['] virtio-serial-close-stdout add-quiesce-xt + \ virtiodev must be shutdown at quiesce so the device is reset properly. + \ The read and write methods can be called after quiesce so must handle + \ virtiodev being closed. + ['] shutdown add-quiesce-xt ; 0 VALUE open-count @@ -62,8 +60,8 @@ virtiodev virtio-serial-init drop open-count 0> IF open-count 1 - dup to open-count 0= IF shutdown THEN + close THEN - close ; : write ( addr len -- actual )