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] [day] [month] [year] [list]

Message-ID: <CALs4sv17Q132-GiCu9pCk9CAS9cpOAF0BVtg0X72jvwmmYRGVg@mail.gmail.com>
Date: Fri, 6 Feb 2026 10:15:57 +0530
From: Pavan Chebbi <pavan.chebbi@...adcom.com>
To: Jason Gunthorpe <jgg@...pe.ca>
Cc: Andy Gospodarek <andrew.gospodarek@...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 Fri, Feb 6, 2026 at 12:12 AM 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.
>

Reading this while I am fully awake helps me grapple with the point
you are making.
Yes, this hole does exist. Thanks for bringing this out.

> IOW this is a *HUGE* security error!
>
> You are going to struggle very much supporting this kind of ill suited
> FW API through this interface, I think you need to rework the FW
> protocol...
>
> Or add some very complex kernel validation and do something about
> forward compatibility...
>
> Or only support commands the FW promises will never have a DMA address
> in them.
>

Or have the driver maintain some sort of look up table where it
validates num_dmas and their offsets for each command it supports, and
reject user commands which don't map...

> Shouldn't you guys have caught this during your security review?
> "don't DMA to random memory" is a very clear fundamental thing in the
> fwctl rules!
>
> Jason

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