From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pg1-f178.google.com (mail-pg1-f178.google.com [209.85.215.178]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 941821E5724 for ; Mon, 20 Apr 2026 12:03:26 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.215.178 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1776686607; cv=none; b=aqx5Fku+05R4LL1MrycR+5QVv6Uhv/epM/QxeeD6I7N71e1+ForTDIkqKPlW1ng1gmq+IxtAnkt2Pr7BKvMHCb6O1zjb28aYzuFeS+uAW/g9ImtRxmlBa2aS7eU735FFqOb8+CW7uMv9nKkiX0wXcKRVZvFLjJERLudNBnF8Fus= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1776686607; c=relaxed/simple; bh=RgQzxQwAcXHwMmn1GP6d9lKNQRGfA7OVpgfu8fgMC8I=; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version; b=QACcXaXdRxnDwwlck3nW3OLzQMCjnzZYVmihVDIr5WJ2cJKwPR7mHIDblAv9T1vI4KYT7EuK5nCTFNoOPWOWS0B7C2XaZcnAto3snALgxR0eTqmw5IZFcBxzb6QMLvzeBAEIKHiNTdjcqFgBTNqS67Ni1MKj2f38t9ILwJz14v0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=BJmPdhXL; arc=none smtp.client-ip=209.85.215.178 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="BJmPdhXL" Received: by mail-pg1-f178.google.com with SMTP id 41be03b00d2f7-c7980c060cfso584263a12.2 for ; Mon, 20 Apr 2026 05:03:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1776686606; x=1777291406; darn=vger.kernel.org; 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=nH+UDp63n0OMToyOF7805ju9M8WJlYdnyXHfyIl0SrE=; b=BJmPdhXLS+BKYVAqyhTjyWt8Yp6YkpkPDmGv5i1GfdiGW1PiBZ59sDDzXJ8WZELoGx D5ZhB21JzcXxZk5faD2ldn7x2OspMhIGhaXOFl4zUPE6Wv2QVSDH79Y7BuKiNubpgpd8 Zp/1F/hkKMcGVurehTgI8IR0mtGQW/2fUz4HJ7nMf8AdOrTMK7RDYrpdoMt3JFCR33qf +Vbyf/ZlhsqRlOgLdzzzE5tb++1h2jo6E2JsC7YWvrDF/bcP+ucL26QlWLldceOKJCr6 WY/f+uQeZRzVKLt6emOK103Y/z2flUxmJCAwhR8nCd0JTuIXSr5+jRh04QSuZWRMGkJp Ms8A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1776686606; x=1777291406; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=nH+UDp63n0OMToyOF7805ju9M8WJlYdnyXHfyIl0SrE=; b=p8uZ6GloawPkIlRNKFiolC5BRfS/eVhH2XFQbufM/kS5X11NDBttfq+i5SbLj57mzr J8rLIoxXkPG+0e0XcIv8Lcj+v4JVBOEF+xGniNQj586NICHKZF33Sw2W+C99HO+DO5eh opl9GEapEPdZTcX63IhCNGoOh3D6I4NsAAh758De+HiO/9gWlQgGaUih59U/JuxJ3VOq BHgokWTqM4UXIUhvG8UWV8JkvfI1P65ULy1AKi02gHbxSuc4LQ1OiJIDZzqjJK5LVtu7 oJu2HC64u6x09NJdYjb1QtJ/85xmln+UH2qG03phoeHAaedmub2aHp/QwHniwVqGprG5 l1Gg== X-Forwarded-Encrypted: i=1; AFNElJ9JM6Q7snvG7BVzjHjsc0eSyCFkomO4DG3ouYLFobJkkjvEM4tW1oF4TtYrgpOIvcjI+FicWuBJOnyrq2k=@vger.kernel.org X-Gm-Message-State: AOJu0YxKTGgDCEKienRd0s4p6q3DHPSuo0kQkC4JCavcWbuiadIfE8O/ RIIiAfR8v+ZIHhtwZyUU1lAEeKSjT526W7YjVzEfVeovpiZRoMoUvvMs X-Gm-Gg: AeBDieuUUHh6wvhXSDCGwufItdMWo3vVipWvf5yLQjrC1CD7FOMJY9A/fZc3sJdqBiK HhyBSy7Y2uitPF6NykKunvdv+k0Wi0Nl7VozY6wEfaFYEVtmkH2IZG3lPAaR/pu5DqdbrYEjhmY V0iDlRzWO7M+bDI/eyhX3Vk1nabhgptUNoHKP0g7wHgawcYkuxXUmJekZFRggZo2NcOZ/yiHT8w wI+JZbzoZyVwRuDYkrEBZkwURFwERqmC4kOXbec5N+7sMHDPDcI/EKenEWddtDqeILbAu/xJ/lU D1WiaBwYwYTMqtYuIUCsc4JtwrccumgCCucmQP4CPXexJHuMrONIGj5jgVewdwE/YZmatQej2nG D2LdN+5gDMzNkzS7V6Rul7Gpnj1MvDZxmMK9ABqO08DoCP3vkOQ2aBq6OqCUEAOudtaQsIE9M4D WzBULyI/FeGb6ZGjDGNEqOEQCXRUmftHAzrpqZ4Ldv+vM= X-Received: by 2002:a05:6a20:a12b:b0:3a2:d79c:414f with SMTP id adf61e73a8af0-3a2d79c440fmr2081566637.48.1776686605729; Mon, 20 Apr 2026 05:03:25 -0700 (PDT) Received: from xiao.mioffice.cn ([43.224.245.230]) by smtp.gmail.com with ESMTPSA id 41be03b00d2f7-c7976f9da4esm7701481a12.11.2026.04.20.05.03.22 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 20 Apr 2026 05:03:25 -0700 (PDT) From: Xiang Gao To: gregkh@linuxfoundation.org Cc: jirislaby@kernel.org, akpm@linux-foundation.org, linux-kernel@vger.kernel.org, linux-serial@vger.kernel.org, gaoxiang17@xiaomi.com, Xiang Gao Subject: Re: [PATCH] sysrq: add optional logging of caller info on /proc/sysrq-trigger write Date: Mon, 20 Apr 2026 20:03:20 +0800 Message-Id: <20260420120320.2137235-1-gxxa03070307@gmail.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <2026041830-visibly-underpaid-6dc8@gregkh> References: <20260416131419.1231012-1-gxxa03070307@gmail.com> <2026041830-visibly-underpaid-6dc8@gregkh> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit On Sat, Apr 18, 2026 at 09:20:14AM +0200, Greg Kroah-Hartman wrote: > We really do not want or like #ifdef in .c files, and for stuff like > this, it is not needed at all. [...] > Module parameters are really not the way for stuff like this. And why > would such a "tiny" option need a config option at all? If you don't > use/need it, it's only a single bool being used? [...] > n is always the default, no need to add it again. Agreed on all of these. The #ifdef, the module_param, the Kconfig entry and the 'default n' all go away -- the feature will be unconditional. Before respinning I would like to align on the approach, because your comments on the kernel log and the ancestry walk touch the core of what this patch should actually be. Let me describe the real use case first, since I do not think I made it clear in v1. Use case - Userspace triggers are typically several processes deep: some service -> library -> system("echo c > /proc/sysrq-trigger") -> sh. The immediate writer is "sh", the actually responsible component is several levels up. - The sysrq character we care about is almost always 'c', 'b' or 'o' -- the machine panics, reboots, or powers off as a direct result of the write. - *After* the device has come back up, we need to identify which userspace component issued the write. Otherwise there is no way to debug why devices in the field are rebooting themselves. > The kernel log is not there for doing audits and the like, so is this > just a debug option? I agree that security audit belongs in the audit subsystem, not in printk. What this patch is trying to do is post-mortem diagnostic for crash/reboot triage, and audit_log() cannot deliver on that: 1. audit_log() is asynchronous -- the record is queued, then delivered via kauditd -> netlink to a userspace subscriber. For sysrq c/b/o the panic/reboot happens before kauditd gets to run, so the record is never delivered. 2. In our deployment environment there is no userspace auditd persisting records to disk, and audit records are not preserved across reboot. The kernel ring buffer, via pstore / ramoops, is the only channel whose contents survive the reboot and are readable on the next boot. pr_info() is the only mechanism that delivers into that persistent path. I would frame this not as audit but as post-mortem diagnostic, retitle the commit accordingly ("sysrq: log triggering task info for post-mortem diagnostics") and drop the audit framing from the changelog entirely. > This might cause problems for when the system is hung and sysrq is the > only way to reboot the box. Have you tried it in that situation? Fair -- the while-loop walking 5 levels has to go. The reason it was there in v1 is the "sh from system()/popen()" case above: the writer's comm is "sh", and the information we actually want is one or more levels up the parent chain. On the hung-system concern specifically: /proc/sysrq-trigger only runs when userspace is alive enough to complete a write(), so the keyboard sysrq path via handle_sysrq() is not affected by this change -- my patch only touches write_sysrq_trigger(). Within that function, rcu_read_lock() / rcu_dereference() take no locks, allocate nothing, and are bounded-constant work, so on paper they should not introduce a new hang. I have not yet reproduced a partially-hung system to verify this empirically; I will do so before v2 and report back. Please correct me if I am misreading the concern. That said, two options I can see, would like your steer before I respin: (a) One bounded rcu_dereference() of current->real_parent. Logs current's pid/comm plus ppid + parent comm. One level, no loop. Covers the direct case app -> system("echo c > /proc/sysrq-trigger") -> sh where sh's parent is the app. (b) Two unrolled rcu_dereference()s -- current->real_parent and that task's real_parent. Still no loop, just two fixed dereferences. Covers the common indirect case service -> library -> system(...) -> sh where sh's parent is the thin library wrapper and the grandparent is the originating service, which is what we actually need to identify in the field. In both cases the RCU read is a fixed number of dereferences, not iteration over a data structure, so the total work is bounded and constant. I lean toward (b): one level is not enough to reach the originating service in the indirect pattern this patch was written for. If two dereferences in the sysrq path are still too much, I will fall back to (a). Which direction would you accept? I will respin based on your answer. Thanks, Xiang