netdev - Re: [PATCH v2 5/6] pds_fwctl: add rpc and query support

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: <20250304172412.GO133783@nvidia.com>
Date: Tue, 4 Mar 2025 13:24:12 -0400
From: Jason Gunthorpe <jgg@...dia.com>
To: Jonathan Cameron <Jonathan.Cameron@...wei.com>
Cc: Shannon Nelson <shannon.nelson@....com>, andrew.gospodarek@...adcom.com,
	aron.silverton@...cle.com, dan.j.williams@...el.com,
	daniel.vetter@...ll.ch, dave.jiang@...el.com, dsahern@...nel.org,
	gregkh@...uxfoundation.org, hch@...radead.org, itayavr@...dia.com,
	jiri@...dia.com, kuba@...nel.org, lbloch@...dia.com,
	leonro@...dia.com, linux-cxl@...r.kernel.org,
	linux-rdma@...r.kernel.org, netdev@...r.kernel.org,
	saeedm@...dia.com, brett.creeley@....com
Subject: Re: [PATCH v2 5/6] pds_fwctl: add rpc and query support

On Tue, Mar 04, 2025 at 05:08:08PM +0800, Jonathan Cameron wrote:
> > +	dev_err(dev, "Invalid operation %d for endpoint %d\n", rpc->in.op, rpc->in.ep);
> 
> Perhaps a little noisy as I think userspace can trigger this easily.  dev_dbg()
> might be better.  -EINVAL should be all userspace needs under most circumstances.

Yes, please remove or degrade to dbg all the prints that userspace
could trigger.

> > +	if (rpc->in.len > 0) {
> > +		in_payload = kzalloc(rpc->in.len, GFP_KERNEL);
> > +		if (!in_payload) {
> > +			dev_err(dev, "Failed to allocate in_payload\n");

kzalloc is already super noisy if it fails

> > +			out = ERR_PTR(-ENOMEM);
> > +			goto done;
> > +		}
> > +
> > +		if (copy_from_user(in_payload, u64_to_user_ptr(rpc->in.payload),
> > +				   rpc->in.len)) {
> > +			dev_err(dev, "Failed to copy in_payload from user\n");
> > +			out = ERR_PTR(-EFAULT);
> > +			goto done;
> > +		}
> > +
> > +		in_payload_dma_addr = dma_map_single(dev->parent, in_payload,
> > +						     rpc->in.len, DMA_TO_DEVICE);
> > +		err = dma_mapping_error(dev->parent, in_payload_dma_addr);
> > +		if (err) {
> > +			dev_err(dev, "Failed to map in_payload\n");
> > +			in_payload_dma_addr = 0;

etc

> > +	err = pds_client_adminq_cmd(pdsfc->padev, &cmd, sizeof(cmd), &comp, 0);
> > +	if (err) {
> > +		dev_err(dev, "%s: ep %d op %x req_pa %llx req_sz %d req_sg %d resp_pa %llx resp_sz %d resp_sg %d err %d\n",
> > +			__func__, rpc->in.ep, rpc->in.op,
> > +			cmd.fwctl_rpc.req_pa, cmd.fwctl_rpc.req_sz, cmd.fwctl_rpc.req_sg_elems,
> > +			cmd.fwctl_rpc.resp_pa, cmd.fwctl_rpc.resp_sz, cmd.fwctl_rpc.resp_sg_elems,
> > +			err);

Triggerable by a malformed RPC?

> > +		out = ERR_PTR(err);
> > +		goto done;
> > +	}
> > +
> > +	dynamic_hex_dump("out ", DUMP_PREFIX_OFFSET, 16, 1, out_payload, rpc->out.len, true);
> > +
> > +	if (copy_to_user(u64_to_user_ptr(rpc->out.payload), out_payload, rpc->out.len)) {
> > +		dev_err(dev, "Failed to copy out_payload to user\n");

Triggerable by a malformed user provided pointer

Jason