From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-qv1-f53.google.com (mail-qv1-f53.google.com [209.85.219.53]) (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 303411E3DCD for ; Fri, 6 Feb 2026 00:39:42 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.219.53 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1770338383; cv=none; b=Kf8ByWMsHDW5KlB2dbrlePAY0WivnvTBSXqdyYAQTRoey594S0wjRD8HE/DW4J/qwFWAj+5dtd4ZuOUkGyAxUUjn0N2Xklst3SzFP+RL47ckUVrlnJ7TAwvWGMx4YNZtEaDt6DsY7W2qSjEXLlrcl/85ppKfQM/imwfbKh/D9O0= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1770338383; c=relaxed/simple; bh=sVZU9kLu6qhmp4WblonLUoIzQTL7nLtmGfoWvXM2iws=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=LZmTSdKvjHZQRn1jUsjPqhHSQGszXdR3Y2AhHv3H4HH217k0yufejNRk3Zh9T9K94f4CeWyAUyWmS9hwsnPmu6KUCdtqBb6N4U7b2DnMsMIk2zMAW/c7+Rj7+LBMSLqasojg/ks0ckv5D2K42QtsLSrmZ5mf7HMfHSewvjZCAuo= 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=CKqkxMW0; arc=none smtp.client-ip=209.85.219.53 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="CKqkxMW0" Received: by mail-qv1-f53.google.com with SMTP id 6a1803df08f44-88a26ce6619so17598756d6.3 for ; Thu, 05 Feb 2026 16:39:42 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ziepe.ca; s=google; t=1770338382; x=1770943182; 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=htMjHKJ/mnbYqJOlrLyHc2rTFGyohzQNIberQXy/W+4=; b=CKqkxMW0z86BeM7IIicG38qwVjy9tSMcbgBtIymMOFSdmgjik/gpmBL/SPqO5Y5pcS 3YZQRJHT61L+DL0vcdUYY3rNTD8w7HJLrFyoNn/6BifP00LwLbWzUhyi9bhFay4n0tCX NeDKvfa1SOnz7tBqspiDkc7P8qBfplx43MgZDX1WRgtrCjnFUz+WDcFhqNUV9Mm/pAEl lA1nktR2jJzVAP+OHb/AqhrJ2K3trWOwhKplkOTN/9n2FXfMzmwu47/n7iGmkZMP4gGz WYD6SEsOeM4fWwNEaVdW5WH0H4YuGD/Iw4EN7dVa48dKxvilq4TmDcfNHs7Nrheaq4qO oiOg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1770338382; x=1770943182; 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=htMjHKJ/mnbYqJOlrLyHc2rTFGyohzQNIberQXy/W+4=; b=YzSSNLrqVmAkQp2RFF7gljgFanYuzB4yLVsiI29b5/o0qud3xQsFdzppxHaTUhJwrz c8mq8ws+juKOzfMDVbrIA2S6+7IMmiIUzlDjKLO48Ql0TalyesJyRvxNCWrtROMOxc0J YtQfFLuCE5qmHPCLO2fpr39f9EE25NdTDatsafIj3++6lDNchmXtjxCHRW+BdN2Kwtvg GKsB4vmJB/Ct19U2TbRSNHNtw9oA5WHD4Apub+5LA8WQdAFmPDUP7ANhVcqUKjfvuvk2 TuMg8UKcnhmJp1EHgjrEZdNa1qWDPMZnyXhniXkVs4yh4lX221GlyVl1elCvhOm7cTZy 9f7Q== X-Forwarded-Encrypted: i=1; AJvYcCWGedYXA0AFmJzjRuGbQX5inRkJPMNeA9dV5NMnHPNf6z8bwzlbMOMPxxdBEAQM4IWUmaMLkxedztgXKy8=@vger.kernel.org X-Gm-Message-State: AOJu0YwsuxrfsfMZ1YVr7lCXxvZG4aX0CxBAgnRgmOia2Y0jcpiUwqER czTYLFRCW/JaKu84SuIV/uuSGWU0R/9voiaL+uv1nSmJEb48Vvcc+Quc5Q9ArbX97IE= X-Gm-Gg: AZuq6aLWmRpoVOGYo1g1QyklANuTAyCluUsQW49RelXfPs+kJopYvvgx8IN6uPcbiY0 vzmD04xE9jLiyCpmaq8tll5KosuZOSqsGL07EUL8LdaPInRfGRg4eQOi+fka4ov0UhL1GT4A90D uBrQJwrL1jvxsNeRmuP39jIVhBSVYkdazFIqxiXBNHqZHhWEPNSzTIZVjwJbKs3FvNO4Yn3HtZI JujYJF0OSOGxM82mPFfVfCCcGj6J9R6oGIw6YYZsU6kGZieYb/gIn6f0hp4jXZS/k9OV7niZO3d 8wLGaz/VFpaWYV5jMhSPCHEgtOvFyZL0WXQD35evbcwTtdhOgHgVYgEgSzm0qzks0bF7xjh7wh5 oF/mIojalrwW+0aafoQTrFB1+5pHhvcaoTZMgqiJlyUtMercF606+Z7gZvRES8y23NUPZZGvXSX uT0loFMYqSRUdE6mhPnxPiEH1RSoaqoa3EEBBG/UOlaBm2ufGBsEgGLDssELhLIbPhKkY= X-Received: by 2002:a05:6214:1c49:b0:889:d7f3:3a5f with SMTP id 6a1803df08f44-8953c7f26e9mr17309286d6.6.1770338381888; Thu, 05 Feb 2026 16:39:41 -0800 (PST) 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 d75a77b69052e-506392edfd7sm5839461cf.30.2026.02.05.16.39.40 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 05 Feb 2026 16:39:40 -0800 (PST) Received: from jgg by wakko with local (Exim 4.97) (envelope-from ) id 1vo9sp-00000003zyg-3jFo; Thu, 05 Feb 2026 20:39:39 -0400 Date: Thu, 5 Feb 2026 20:39:39 -0400 From: Jason Gunthorpe To: Andy Gospodarek Cc: Pavan Chebbi , Saeed Mahameed , michael.chan@broadcom.com, linux-kernel@vger.kernel.org, dave.jiang@intel.com, Jonathan.Cameron@huawei.com, gospo@broadcom.com, selvin.xavier@broadcom.com, leon@kernel.org, kalesh-anakkur.purayil@broadcom.com Subject: Re: [PATCH v3 fwctl 4/5] fwctl/bnxt_fwctl: Add bnxt fwctl device Message-ID: <20260206003939.GA943673@ziepe.ca> References: <20260129155453.3626544-1-pavan.chebbi@broadcom.com> <20260129155453.3626544-5-pavan.chebbi@broadcom.com> <20260205173346.GO2328995@ziepe.ca> <20260205184206.GP2328995@ziepe.ca> 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: On Thu, Feb 05, 2026 at 07:19:43PM -0500, Andy Gospodarek wrote: > On Thu, Feb 5, 2026 at 1:42 PM Jason Gunthorpe 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. I think it is pretty clear, the DMA addreses are being read by FW from the same memory buffer that the userspace fully controls. Look at the code, the design takes an offset list from userspace and patches in DMA addresses from the kernel into the same buffer that the userspace data was copied from. NOTHING is checking that the offset list is security acceptable for the command!! > 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. I understand how the buffer copying works on the happy path, I am pointing out that if the user submits a command that has +struct fwctl_rpc_bnxt { + __aligned_u64 req; + __u32 req_len; + __u32 timeout; + __u32 num_dma; ^^^^^^^^^^^ === 0 then the kernel doesn't even allocate any of these DMA buffers, it doesn't change the command buffer and it sends raw userspace data directly to FW. Since that very same command buffer obviously contains the DMA addresses to use there is nothing blocking userspace from doing this. Stated another way, the kernel must NOT take in +struct fwctl_dma_info_bnxt { + __aligned_u64 data; + __u32 len; + __u16 offset; ^^^^^^^ This parameter from userspace! The kernel MUST know exactly where all DMA addresses lie in the message now and into the future to guarentee that it and only it writes a DMA address of the kernel controlled DMA bounce buffer. Allowing a userspace controlled DMA address to pass into the device directly from userspace is a complete security fail. Jason