linux-kernel - Re: [PATCH v3 fwctl 4/5] fwctl/bnxt

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CACDg6nV4Dfc+ggiMmsfQWJtfO1CuHaqps25NdmBgkYuMYv8Tcw@mail.gmail.com>
Date: Thu, 5 Feb 2026 19:19:43 -0500
From: Andy Gospodarek <andrew.gospodarek@...adcom.com>
To: Jason Gunthorpe <jgg@...pe.ca>
Cc: Pavan Chebbi <pavan.chebbi@...adcom.com>, Saeed Mahameed <saeedm@...dia.com>, 
	michael.chan@...adcom.com, linux-kernel@...r.kernel.org, dave.jiang@...el.com, 
	Jonathan.Cameron@...wei.com, gospo@...adcom.com, selvin.xavier@...adcom.com, 
	leon@...nel.org, kalesh-anakkur.purayil@...adcom.com
Subject: Re: [PATCH v3 fwctl 4/5] fwctl/bnxt_fwctl: Add bnxt fwctl device

On Thu, Feb 5, 2026 at 1:42 PM Jason Gunthorpe <jgg@...pe.ca> wrote:
>
> On Thu, Feb 05, 2026 at 12:47:53PM -0500, Andy Gospodarek wrote:
>
> >    Jason, this is all done in bnxtctl_fw_rpc() and the functions it calls
> >    in this (or the v4 version) of this patch.
>
> Oh.. No.. You can't do this:
>
> +       rpc_in.msg = memdup_user(u64_to_user_ptr(msg->req), msg->req_len);
>          // ^^ eventually becomes fw_msg
> +               dma_ptr = fw_msg->msg + msg->offset;
> +               *(__le64 *)(dma_ptr) = cpu_to_le64(dma_addr[i]);
>
> There is nothing in here which ensures that every DMA physical address
> in the mailbox given from userspace is *actually forced to a value by
> the kernel*
>
> The kernel only overwrites values from the user controlled struct
> fwctl_dma_info_bnxt array.
>
> Meaning userspace can just specify any physical address in the
> message, not include any fwctl_dma_info_bnxt list and it will happily
> send the user controlled physical address to the FW. Thus userspace
> can DMA to whatever memory it likes.
>
> IOW this is a *HUGE* security error!
>

I'll let Pavan chime in when he sees this, but I'm not sure that's
correct.  You might be right that we have a problem that is as bad as
you describe, but it's also possible we could do a better job
explaining how these DMA operations work and why they are needed.

Not every FW RPC will need to have chunks of data that is copied to or
from userspace to FW.  In the cases where data is copied, there is not
a direct DMA from userspace to FW or vice-versa.  All DMA operations
should be done on buffers only allocated and mapped with
dma_alloc_coherent -- never buffers or addresses that are straight
from userspace.

As I read through these again, I realize that what might have made
these easier to read would be simply changing the names of a few of
these structures when they are really just buffers being copied
between user and kernel space -- not buffers that are actually being
DMAed from hardware to userspace.

Download attachment "smime.p7s" of type "application/pkcs7-signature" (5489 bytes)