From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-qk1-f179.google.com (mail-qk1-f179.google.com [209.85.222.179]) (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 F1E0A2773E5 for ; Fri, 13 Mar 2026 16:14:20 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.222.179 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1773418462; cv=none; b=k/N48ZEahZCvXBtrI6vV7bx9iaHHKQYFZ839qeVaAqNuubFuzflQpuAyYsFCSQyc6pb0g+4VEzIVEFIKBqJEpxfj7qtrGALhFVp6maR+gVXoVB3V6jZcsZWJQWe9KyIuXCZINWKQ+jzWP2Q02GOnlH/h2pr/7W3AiajP3Cp4zJc= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1773418462; c=relaxed/simple; bh=V7cMtslV82xkE3l6Ng6kelQkXqcYJpbU8YzivEVMK6E=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=WV352djiRi4UROn6l7MX9cOymzQbJXOCJESz7/6m/Mm3w2+nSiNInGGpy3TBmvYtnqU3ARi6SZqYvbt1XVXt1N+mCHgqr2+8Gaw3aHP9jW4mDqL1palGoquV3p12vN0SYw9PcsnPx7s5Q5EsLhlHSldaAZBN2mw8x+gjKEdNAQA= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=ziepe.ca; spf=pass smtp.mailfrom=ziepe.ca; dkim=pass (2048-bit key) header.d=ziepe.ca header.i=@ziepe.ca header.b=MmN2/PkF; arc=none smtp.client-ip=209.85.222.179 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=ziepe.ca Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=ziepe.ca Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=ziepe.ca header.i=@ziepe.ca header.b="MmN2/PkF" Received: by mail-qk1-f179.google.com with SMTP id af79cd13be357-8cd767d2d70so243342685a.3 for ; Fri, 13 Mar 2026 09:14:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ziepe.ca; s=google; t=1773418460; x=1774023260; darn=vger.kernel.org; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:from:to :cc:subject:date:message-id:reply-to; bh=kjYg1aEsgP1/bS16W9kG1pNjIPB7lNkt3fG9Y0+BLzo=; b=MmN2/PkFR1dFPnBl7BEM1N4eGr8F5gncMadBM1OvwDjvqOpbhhOsw99HgPyxYnHmG3 NH0xpwqT6Kz7VMQhVGul9wmU8dWCeqizMH8GINtlY7MNCcjEu3R7eq1YrM0bzNJ3cxHh JXOuhknofANI5zQDcHiCmCcIFAl0KOg5ufNoXSpWlHuAslQ/ZcuN9j5PXTE7oVFROwzj oEKTyzBdgUiq7yAPUy1j6dDAbkQNbT3bqcrmMj/CbKpGF3++4l/X+o7D+D25OdtGlToe bCEr04657zikyPF6/04DJOef8QYaV3I5ptiP8RHRyMIxTAoQHVzRaRG4dv0qLZZtDCV5 au4Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1773418460; x=1774023260; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=kjYg1aEsgP1/bS16W9kG1pNjIPB7lNkt3fG9Y0+BLzo=; b=WgnzUUPTVysE3SjZPtr3LE2N4CqYZc5NvyjVq0njgJ0yqXhUdqFuT4NC/8vL3x8BNE wnPCh7iN7m+i7YXgBq0m7tA9LD1iOwz4DsY37YsMylt+bJAhRKdeg4TVGfkaZG7j8teN 6VXsI/SidP/q9CToAYG8Ky7aPAsxBZ8oYKZIhs8huifes3DQlYclSiDh+x2I7uZ5UaAQ 9ELQynmH/5PV6+ptXNbd/fUSKck3WDRnGUwxl1l7tph5gv3fF5hLApEjlelen7jIqSrV 55FFSSQq3aOKk0Van9fUNzEj8BP3AmxoeRWjRhZINIE7kMNpu9LprKnyVuoC1/voWNsq NlQA== X-Forwarded-Encrypted: i=1; AJvYcCWWoQBeose1nwI2zrYHOeI7cnDjireIogAMK0N2bBatcAhtCKRZft0TvKA0kgHln/iOiYqX2uGBoBNjMw0=@vger.kernel.org X-Gm-Message-State: AOJu0Yw7WMFomyxU/88z8qkLBJyqMlE/vOJ0ULXuzcWGbSiE8JcOqvJu kRqpg1XJ8peh8PvwcBIeRY0FW2m20kEeFp8XRSejE2ya6t9is25Aqs4raAV2pxvg+6c= X-Gm-Gg: ATEYQzx+ZPGOTR7RwE2eIIkbZSs8kTwYM2+EINLu/kIWiaZopCkffADoCUnZ1hW4aSP FoaR8fC50oLMUca7+LLNoBK68nu8/qymMIyTpC9pFe7/J62wz0ZK1FP0jFMg0wYDl3hN0VyEhbL wIA74oYjiP08y454oB2teTQcOCc8hRtsWfoMUFRuvJlfPXB5yfQ6asns+Fgm4AfuAZnTeNNUSVf FFXfkmACaalcNPqJ4+xOeS59laFukBk7L56zPlqelpg7yA4KdQTYQj++xKJnb1u8Jw3JYzHsyWO caOQO0lqZl4cy3/A35P59n+2s80P0z93lA1ZwPTD7Ci+0DefDS8syq7NJ6v8qSqb5DukhFdbbxS eivjFXvAPBoFRA2qQYIT0FqpEFNIcJ09F8ywpcoM64HQwgQ5DXHqGfuQR9qaNSdEcdrIgbsdnE0 YmFByolugDyFIH/Ob9ET/GD35Euirs9KQj6v4zwdNoFiKuQF5ggLCp7SX8H1Efjfo0MoQjCDZhj 7hnwmRlcqJ+ApnZFsQ= X-Received: by 2002:a05:620a:4629:b0:8c6:a5bb:f464 with SMTP id af79cd13be357-8cdb5ba1b99mr551610285a.66.1773418459802; Fri, 13 Mar 2026 09:14:19 -0700 (PDT) Received: from ziepe.ca (hlfxns017vw-142-162-112-119.dhcp-dynamic.fibreop.ns.bellaliant.net. [142.162.112.119]) by smtp.gmail.com with ESMTPSA id af79cd13be357-8cda1fc474dsm663339785a.6.2026.03.13.09.14.18 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 13 Mar 2026 09:14:19 -0700 (PDT) Received: from jgg by wakko with local (Exim 4.97) (envelope-from ) id 1w159W-00000007IgL-1gE1; Fri, 13 Mar 2026 13:14:18 -0300 Date: Fri, 13 Mar 2026 13:14:18 -0300 From: Jason Gunthorpe To: Pavan Chebbi Cc: michael.chan@broadcom.com, linux-kernel@vger.kernel.org, dave.jiang@intel.com, saeedm@nvidia.com, Jonathan.Cameron@huawei.com, gospo@broadcom.com, selvin.xavier@broadcom.com, leon@kernel.org, kalesh-anakkur.purayil@broadcom.com Subject: Re: [PATCH v5 fwctl 4/5] fwctl/bnxt_fwctl: Add bnxt fwctl device Message-ID: <20260313161418.GC1704121@ziepe.ca> References: <20260226082318.525518-1-pavan.chebbi@broadcom.com> <20260226082318.525518-5-pavan.chebbi@broadcom.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=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20260226082318.525518-5-pavan.chebbi@broadcom.com> On Thu, Feb 26, 2026 at 12:23:17AM -0800, Pavan Chebbi wrote: > + rpc_in.msg_len = msg->req_len; > + rpc_in.resp = kzalloc(*out_len, GFP_KERNEL); > + if (!rpc_in.resp) { > + err = -ENOMEM; > + goto free_msg_out; > + } > + > + rpc_in.resp_max_len = *out_len; > + if (!msg->timeout) > + rpc_in.timeout = FWCTL_BNXT_HWRM_DFLT_TIMEOUT; > + else > + rpc_in.timeout = msg->timeout; > + > + rc = bnxt_send_msg(bnxt_aux_priv->edev, &rpc_in); > + if (rc) { > + struct output *resp = rpc_in.resp; > + > + /* Copy the response to user always, as it contains > + * detailed status of the command failure > + */ > + if (!resp->error_code) > + /* bnxt_send_msg() returned much before FW > + * received the command. > + */ > + resp->error_code = rc; > + } > +free_msg_out: > + kfree(rpc_in.msg); Timeout is such a complicated thing to add to a HW RPC interface. Does bnxt do it right? Claude says no, and it looks compelling to me.. So don't give userspace an easy ability to trigger timeout, by lowering the timeout value, and causing corruption in the kernel. If you hit a FW timeout you probably have to FLR the device to recover from it, if fwctl is going to have a timeout knob it has to leave the request active in the kernel and orphan it to avoid a FLR - not sure that makes much sense. >> I want to investigate what the state is after this function fails, >> particularly if it fails for a timeout. Is there any cahnce that >> fw_msg->resp can be written too after the function returns because >> it is still programmined into HW as a DMA? DMA Safety Analysis: bnxt_send_msg() Timeout Path Overview Investigation of whether firmware can DMA into a freed buffer after bnxt_send_msg() returns due to a timeout. How resp_addr Gets Programmed Into HW At bnxt_hwrm.c:90, the DMA address of the response buffer is embedded in the request message itself: ctx->req->resp_addr = cpu_to_le64(dma_handle + BNXT_HWRM_RESP_OFFSET); This address is sent to the firmware as part of the HWRM command. The firmware caches it and will DMA the response to that address when it completes. What Happens on Timeout There are multiple timeout goto exit paths (lines 587, 645, 668 in bnxt_hwrm.c). All jump to the exit label at line 685: exit: if (token) __hwrm_release_token(bp, token); if (ctx->flags & BNXT_HWRM_INTERNAL_CTX_OWNED) ctx->flags |= BNXT_HWRM_INTERNAL_RESP_DIRTY; else __hwrm_ctx_drop(bp, ctx); No abort command is sent to the firmware. No mechanism exists to tell the HW to stop using resp_addr. The Specific Flow in bnxt_send_msg() Since hwrm_req_hold() is called, the request is “owned”, so on timeout: 1. __hwrm_send() sets BNXT_HWRM_INTERNAL_RESP_DIRTY flag and returns error. 2. Back in bnxt_send_msg(), resp_len is read from resp->resp_len. On a timeout this is likely 0 (buffer was zeroed at alloc), so the memcpy is skipped. 3. hwrm_req_drop() calls __hwrm_ctx_drop() which calls dma_pool_free() at line 313. The DMA Hazard After dma_pool_free(), the buffer goes back to the pool. But the firmware still has resp_addr and may still DMA the response to it. This means: 1. The pool buffer can be reallocated for a new HWRM request via dma_pool_alloc(). 2. The firmware can write to it at any time – corrupting the new request’s data or response. The BNXT_HWRM_INTERNAL_RESP_DIRTY flag only causes a memset on the next use of the same buffer (around line 476), but that doesn’t help – it’s a race. The firmware DMA can arrive after the memset, after the new request is in flight, or even after the new response is written. Regarding fw_msg->resp Specifically In bnxt_send_msg() itself, fw_msg->resp is not a DMA target – it’s a plain kernel buffer that gets memcpy’d into. So fw_msg->resp won’t be directly corrupted by late DMA. The DMA target is the internal ctx->resp buffer from the pool. Summary ----------------------------------------------------------------------- Aspect Finding ----------------------------------- ----------------------------------- fw_msg->resp written by late DMA? No – it’s a memcpy destination, not a DMA target Pool DMA buffer at risk? Yes – firmware retains resp_addr after timeout Firmware abort on timeout? No – no abort/stop mechanism exists DMA unmap on timeout? No – dma_pool_free() returns buffer to pool without unmapping Synchronization barrier? No – no barriers or delays before freeing Corruption scenario Firmware completes late, DMA writes into reallocated pool buffer used by a new HWRM command -----------------------------------------------------------------------