From patchwork Sun Nov 27 15:45:59 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Thomas_Wei=C3=9Fschuh?= X-Patchwork-Id: 26381 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:adf:f944:0:0:0:0:0 with SMTP id q4csp5186684wrr; Sun, 27 Nov 2022 08:12:28 -0800 (PST) X-Google-Smtp-Source: AA0mqf6J6QQbMOdWS0NLjZn6+a/w8XiE+Dln6ZwXG/R2qNiGxGFfUpIc13+ont7oUmaM3+ErfgHD X-Received: by 2002:a17:90b:1e0f:b0:213:c5ae:55ec with SMTP id pg15-20020a17090b1e0f00b00213c5ae55ecmr48839548pjb.182.1669565548610; Sun, 27 Nov 2022 08:12:28 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1669565548; cv=none; d=google.com; s=arc-20160816; b=iP4TCQPZD6lTbKAlzCSp7MbPrk2qXQOj+/LQm5zRImIvXY6M0xFzb//LGHIQ1In0EV R1A8VDRziu+/DPzBLYXNxbDQdJUdM3gi9fjAhoAe7k6uRBVik6RO39d4IYUv4qTyNkkX Vz4gFBVW6G+QppPkTHBTHKzR4Vq4eoaGz3mz7X0YaXuub8sb+HUWNzy+rpgO49xeZQ2Q DCCw+8pTMQsl0/jz+bfl3DrxK6ANiNBpU3lKdcvudReyzBoCC2oODsTwp/RPmKr9G5An EqWL5nmKzHTPnoEsQSNIUUVFqc9okuzVsZ9vhNEdIMl6ao09/GfS/0TQYTGb57FrSwan DQsA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:dkim-signature:from; bh=/K0qioOIZ3R6eNt0k1QFJ55p7+p0nHn6PtBVATjZ0vQ=; b=jlooHaC4KGFWUlhqQh1oHuacXMXOxHYMJqbty030PBdJMvsM97MpAYV2L6zPLz/fSW 8vw7DxcQYCvzK+9xWyy3958XcNLrhGdjQGiMPMoAg3kAymYKkQiUKm0BY/t3m5KtAJH+ LQ8bGcBzizRH0gfqbsNsEYnLIQIFHBTuwlP3YiLiz1Sm7PQTI9yVTyZeUQE1SrSVXIVB ZIxOojjx8Tbh4ra6qEz3VwosoOzg0WNBeaPTW1rx+wE5vj8AB2FOerz9vtRRrdBzrX4V 21RhzWGQEkBCNSbiVOecfJolvTb6Xp3D2wGgRhQffcq5tkCZEBIWwHErGMqssENr+a/z xSeQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=fail header.i=@weissschuh.net header.s=mail header.b=OOzbe3dl; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id np2-20020a17090b4c4200b002130d0f6df8si9832693pjb.30.2022.11.27.08.12.16; Sun, 27 Nov 2022 08:12:28 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=fail header.i=@weissschuh.net header.s=mail header.b=OOzbe3dl; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229618AbiK0PyV (ORCPT + 99 others); Sun, 27 Nov 2022 10:54:21 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:56918 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229521AbiK0PyU (ORCPT ); Sun, 27 Nov 2022 10:54:20 -0500 Received: from todd.t-8ch.de (todd.t-8ch.de [IPv6:2a01:4f8:c010:41de::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id EF760DEC0; Sun, 27 Nov 2022 07:54:18 -0800 (PST) From: =?utf-8?q?Thomas_Wei=C3=9Fschuh?= DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=weissschuh.net; s=mail; t=1669564456; bh=hFVNL9asli5tIDyA3WxJjlkOG1f2F7NESUl0mhNd6YQ=; h=From:To:Cc:Subject:Date:From; b=OOzbe3dlF1Uxr9eSIlFbZU84yF0v4K1jNDuIEgVUoAqvQpbIUfpRLeBBpvzfde4Uo CxMNU8qyRePDw3E7icl5wkBNTOJUezOV77+ie5w2NzfMMAxb0B9pT8L3/JGu9XdGI9 kJMC++xE2v4RxckNRdI59ScIVKrnVg1SjMkPwlbQ= To: Wim Van Sebroeck , Guenter Roeck Cc: =?utf-8?q?Thomas_Wei=C3=9Fschuh?= , linux-watchdog@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [RFC PATCH] watchdog: core: don't reset KEEPALIVEPING through sysfs Date: Sun, 27 Nov 2022 16:45:59 +0100 Message-Id: <20221127154559.80899-1-linux@weissschuh.net> X-Mailer: git-send-email 2.38.1 MIME-Version: 1.0 X-Developer-Signature: v=1; a=ed25519-sha256; t=1669563958; l=4246; i=linux@weissschuh.net; s=20211113; h=from:subject; bh=hFVNL9asli5tIDyA3WxJjlkOG1f2F7NESUl0mhNd6YQ=; b=aMGG2Q+p8KJE7dtKXkBnNnJl5JcuNK/70jYbUBzbB3ynzcO04qDChCmSESJDF7QwKQVUdSP8wLtJ Zy1JDGSLBn/HciFPOrL/WFhNvBbBCDFS4oBtbI088sEQd0YBeon5 X-Developer-Key: i=linux@weissschuh.net; a=ed25519; pk=9LP6KM4vD/8CwHW7nouRBhWLyQLcK1MkP6aTZbzUlj4= X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1750666365082801235?= X-GMAIL-MSGID: =?utf-8?q?1750666365082801235?= Reading the watchdog status via ioctl(WDIOC_GETSTATUS) or sysfs will reset the status bit KEEPALIVEPING. This is done so an application can validate that the watchdog was pinged since the last read of the status. For the ioctl-based interface this is fine as only one application can manage a watchdog interface at a time, so it can properly handle this implicit state modification. The sysfs "status" file however is world-readable. This means that the watchdog state can be modified by any other unprivileged process on the system. As the sysfs interface can also not be used to set this bit, let's remove the capability to clear it. Fixes: 90b826f17a4e ("watchdog: Implement status function in watchdog core") Signed-off-by: Thomas Weißschuh --- I was not able to find an application (besides wdctl) that uses this bit. But if applications are to use it, it should probably be reliable. Other possible solutions I can think of: * Only reset the bit when the file opened privileged * Only reset the bit when the file opened writable Instead of using a status bit to check if the watchdog was pinged it would probably be more robust to use a sequence counter or timestamp. Especially as it seems more operations are being exposed over sysfs over time. I'm not sure it's worth it though. --- Documentation/ABI/testing/sysfs-class-watchdog | 3 ++- drivers/watchdog/watchdog_dev.c | 13 +++++++++---- 2 files changed, 11 insertions(+), 5 deletions(-) base-commit: faf68e3523c21d07c5f7fdabd0daf6301ff8db3f diff --git a/Documentation/ABI/testing/sysfs-class-watchdog b/Documentation/ABI/testing/sysfs-class-watchdog index 585caecda3a5..182a8a9e530e 100644 --- a/Documentation/ABI/testing/sysfs-class-watchdog +++ b/Documentation/ABI/testing/sysfs-class-watchdog @@ -38,7 +38,8 @@ Contact: Wim Van Sebroeck Description: It is a read only file. It contains watchdog device's internal status bits. It is equivalent to WDIOC_GETSTATUS - of ioctl interface. + of ioctl interface, except that the status bit + WDIOF_KEEPALIVEPING is not reset. What: /sys/class/watchdog/watchdogn/timeleft Date: August 2015 diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c index 55574ed42504..7e3e964c4d6f 100644 --- a/drivers/watchdog/watchdog_dev.c +++ b/drivers/watchdog/watchdog_dev.c @@ -320,13 +320,15 @@ static int watchdog_stop(struct watchdog_device *wdd) /* * watchdog_get_status - wrapper to get the watchdog status * @wdd: The watchdog device to get the status from + * @reset_keepaliveping: Reset the status bit _WDOG_KEEPALIVE * * Get the watchdog's status flags. * The caller must hold wd_data->lock. * * Return: watchdog's status flags. */ -static unsigned int watchdog_get_status(struct watchdog_device *wdd) +static unsigned int watchdog_get_status(struct watchdog_device *wdd, + bool reset_keepaliveping) { struct watchdog_core_data *wd_data = wdd->wd_data; unsigned int status; @@ -345,9 +347,12 @@ static unsigned int watchdog_get_status(struct watchdog_device *wdd) if (test_bit(_WDOG_ALLOW_RELEASE, &wd_data->status)) status |= WDIOF_MAGICCLOSE; - if (test_and_clear_bit(_WDOG_KEEPALIVE, &wd_data->status)) + if (test_bit(_WDOG_KEEPALIVE, &wd_data->status)) status |= WDIOF_KEEPALIVEPING; + if (reset_keepaliveping) + clear_bit(_WDOG_KEEPALIVE, &wd_data->status); + if (IS_ENABLED(CONFIG_WATCHDOG_HRTIMER_PRETIMEOUT)) status |= WDIOF_PRETIMEOUT; @@ -476,7 +481,7 @@ static ssize_t status_show(struct device *dev, struct device_attribute *attr, unsigned int status; mutex_lock(&wd_data->lock); - status = watchdog_get_status(wdd); + status = watchdog_get_status(wdd, false); mutex_unlock(&wd_data->lock); return sysfs_emit(buf, "0x%x\n", status); @@ -753,7 +758,7 @@ static long watchdog_ioctl(struct file *file, unsigned int cmd, sizeof(struct watchdog_info)) ? -EFAULT : 0; break; case WDIOC_GETSTATUS: - val = watchdog_get_status(wdd); + val = watchdog_get_status(wdd, true); err = put_user(val, p); break; case WDIOC_GETBOOTSTATUS: