From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from fout-a3-smtp.messagingengine.com (fout-a3-smtp.messagingengine.com [103.168.172.146]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id D5A013B389A for ; Mon, 15 Jun 2026 11:11:26 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.146 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1781521889; cv=none; b=QQ1Tc0CwOqGfPJkD4q63VKXMmzSLciafxVbIV02VEPry2gxq0FsjlpPHykPvEOpF8OlXuYdRl4bhu2iOwAqWWHSZ8tV958MNS5o3Xkdu93llZxSoEuBZhLip8pSfg4WIGNn1wgxcZDxAKygFCZTq5RSC/68DwxaJmiJZQp7bnZI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1781521889; c=relaxed/simple; bh=6p0wxdraLOr0BCtRq0MXQsgll+bU4f1LJ9hQWHpQh1A=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=OK1sySKVWowRvX8SYldHF7a99xf4euBDb6+o1wCFgZ9rbYfa0iUVPkfPN3l3bje/1PgS2VmLcXCnXdTfT6kBkdbeFFbbrotvqbggyW8g4oEa0r5Zoan3oZp+d2R/FCtXldGNNKSG0uxQbAwdqLk94JiUNi9Uai7ceRqHL3VJeIU= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=sakamocchi.jp; spf=pass smtp.mailfrom=sakamocchi.jp; dkim=pass (2048-bit key) header.d=sakamocchi.jp header.i=@sakamocchi.jp header.b=DtwZTX4X; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=OGuyYcsb; arc=none smtp.client-ip=103.168.172.146 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=sakamocchi.jp Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=sakamocchi.jp Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=sakamocchi.jp header.i=@sakamocchi.jp header.b="DtwZTX4X"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="OGuyYcsb" Received: from phl-compute-09.internal (phl-compute-09.internal [10.202.2.49]) by mailfout.phl.internal (Postfix) with ESMTP id D9551EC00CE; Mon, 15 Jun 2026 07:11:25 -0400 (EDT) Received: from phl-frontend-04 ([10.202.2.163]) by phl-compute-09.internal (MEProxy); Mon, 15 Jun 2026 07:11:25 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sakamocchi.jp; h=cc:cc:content-type:content-type:date:date:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:subject:subject:to:to; s=fm1; t=1781521885; x= 1781608285; bh=cgKUpaGRb2wpys10AuxMybs9Um/6OJXsJbtlPE9YwPY=; b=D twZTX4X5gv9gukjY5elgWxlTN1Sn/0v/DIDbvJj3rpRzxvjoKPe9kcFwsGpDMTAR ChM5ysCfEVsLAgK3JTGvro75OJerBpWF6QQk76l7gK/RI6+EnNSn7ZhpfT12vRxc jf3MhA9va0kK+GE9R1cLTafUsGLZs/hhvqCRrk/T+tv41D1EBAH5Rei9TNLNITs5 DKAEG/23vj70BMgULEXNHwumnrfhAv4z8uqzLX8EiB3MahTy40gMUB4sgnIO6kKy 4bd0dcRmqrBvUk5kVqBzjGArkiMQrOrRAvz1pIXezifwbGx54YkKt8wOmfIqpcrO tOXkFCLhQAucZ8KKio1oQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:subject:subject:to :to:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm1; t= 1781521885; x=1781608285; bh=cgKUpaGRb2wpys10AuxMybs9Um/6OJXsJbt lPE9YwPY=; b=OGuyYcsbeK6OB63YXZkFKKH3hRZHcHY4qZJXUDg62A1d8W07Ezl qTzDO50jtqkN07ApoyzozI5of8KFwoHuA1iXOz+XF0lvfk7RcjXLEWC2n21cI0aC IjQX8a5kHaw2FV+H+PC7R7qi0u90E6uS6EMpIgw25usB8RYk+LKSeFGZ90t2Vtvf 4sh4e3pnlTyK8mFKIDs+Kn9ljVjXcekq7aQm9s9D2M/044LaN9/LUjfJQaSM1zPF Da4uR++B+4U7/opLp/v8vM5xjQLRCGBPcKd78nFq+beQi6A2iVWigpyXAIijyge0 FOcXsg2Nh9q89cPgGCibLl9cy/UY7XIq8bA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: dmFkZTGY1imVDeAkB+N1osQtLl6E+7PvzJ4hvoRxVkiP1lxV129SIHhJXmqreVGTto5ulV 60AmVP5owdVyEN9O99Q8oHseH02AaaFjbgC5nzAK6n2vgmQcTRCLnrbC3nyNQt2Mex6Oqd dQXlRrhslQR9FOgSB0g7wvQxquTiJ/MRIyvwf61vfQBBKJBkOTsL48AD0DW0AzEt91mfie lq6+Eamkkrm7/8RGiPa21uDA0k3sriym0rvd526637Sp2FPuhQlN8dv018W+BCgnJoD1Bf /OYRuNkxtVfDhgXmo/UZv4l15aOV7xpfRpJnmunmxTM3t+WbtQb6y8IXYujPOeDAwUSas6 87Otd62H4dOplcX2q9HtwTDVphyM8TY3X+e6o+XoOL4iIWw08Lh9xJ1acF4FLA2sb8mNcZ sSfHla6phPTHWB9Yv5O2RqQ6gjRpcqSKWR0CaTfW86n6Wa3M6OoaRKhYNsBlOyewNxDwzn Ur/m2U/6mMsNLEZ7n6H7/k0Hs9jJT31d+Z0Qoh2YuQVDsyOjPTP2ZqGsPVxnrUvVcPyOa4 Iw7hJKXxdY5cUT8fVaU19HR4l6FuwBO37pZ9bmI393FM/0HLX8PCDQwVI613yI7vi/dPe9 QQqfWIRybNtyRPLYI0ViAV+n91ulVKA3S18/ZsBC/BnG7PNm4woja/UTKWYw X-ME-Proxy: Feedback-ID: ie8e14432:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 15 Jun 2026 07:11:24 -0400 (EDT) Date: Mon, 15 Jun 2026 20:11:22 +0900 From: Takashi Sakamoto To: Michael Bommarito Cc: linux1394-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org Subject: Re: [PATCH] firewire: core: propagate cycle-timer read failure in legacy ioctl Message-ID: <20260615111122.GA176369@sakamocchi.jp> Mail-Followup-To: Michael Bommarito , linux1394-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org References: <20260614151543.2976015-1-michael.bommarito@gmail.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260614151543.2976015-1-michael.bommarito@gmail.com> Hi, Thanks for the patch, however I can see some subtle gotchas. On Sun, Jun 14, 2026 at 11:15:43AM -0400, Michael Bommarito wrote: > The legacy FW_CDEV_IOC_GET_CYCLE_TIMER handler ioctl_get_cycle_timer() > reads into a local struct fw_cdev_get_cycle_timer2 via > ioctl_get_cycle_timer2() and then copies tv_sec, tv_nsec and cycle_timer > into the user-visible output, but it ignores the helper's return value. > Since commit bacf921c42bb ("firewire: core: use guard macro to disable > local IRQ") the helper returns early on fw_card_read_cycle_time() > failure (for example -ENODEV after the dummy card driver is swapped in > during controller removal) without writing those fields, so the wrapper > copies out the uninitialized local: 12 of the 16 user-visible bytes are > uninitialized kernel stack. The non-legacy ioctl already returns the > error on this path. > > Propagate the helper return value so no output is copied on failure. > > Reproduced under KMSAN; the legacy wrapper no longer returns > uninitialized cycle-timer bytes after this change. > > Fixes: bacf921c42bb ("firewire: core: use guard macro to disable local IRQ") > Cc: stable@vger.kernel.org # v6.12+ > Assisted-by: Claude:claude-opus-4-8 > Signed-off-by: Michael Bommarito > --- > Reproduction (v7.1-rc4, x86_64 QEMU, CONFIG_KMSAN=y, stack auto-init off): > > With fw_card_read_cycle_time() forced to fail, the stock legacy handler > returns 0 and KMSAN reports the uninitialized local being stored to the > output: > > BUG: KMSAN: uninit-value in ioctl_get_cycle_timer > Local variable ct2 created at: ioctl_get_cycle_timer > > The leaked output is local_time (8 bytes, bytes 0-7, computed from the > uninitialized tv_sec/tv_nsec) and cycle_timer (4 bytes, bytes 8-11, > copied verbatim). After this change the handler returns -ENODEV and the > KMSAN reports are gone; a success-path control stays clean. > > drivers/firewire/core-cdev.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) The issued function, ioctl_get_cycle_timer() was introduced by a commit abfe5a01ef1e ("firewire: cdev: add more flexible cycle timer ioctl")[1]. We can see in this time the function has already ignored the return value of get_cycle_timer2(). The issue is independent of the later commit bacf921c42bb ("firewire: core: use guard macro to disable local IRQ"). Besides, userspace applications can unbind 1394 OHCI driver in any time, so it is possible to use the vulnerability reading small part of kernel stack. (Understandably, if the attacker is successful to install 1394 OHCI device to the target hardware, he or she can accomplish more jobs regardless of the issue.) Anyway, the leak of kernel stack to userspace is not preferable itself. I would like to see your revised patch, writing on your own. [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=abfe5a01ef1e4 Thanks Takashi Sakamoto