linux-kernel - Re: [RFC PATCH 06/14] usb: core: hcd: Introduce USB HCD APIs for interrupter management

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: <Y6a+VJ75lRIUE9yD@kroah.com>
Date:   Sat, 24 Dec 2022 09:54:44 +0100
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Wesley Cheng <quic_wcheng@...cinc.com>
Cc:     srinivas.kandagatla@...aro.org, mathias.nyman@...el.com,
        perex@...ex.cz, broonie@...nel.org, lgirdwood@...il.com,
        andersson@...nel.org, krzysztof.kozlowski+dt@...aro.org,
        Thinh.Nguyen@...opsys.com, bgoswami@...cinc.com, tiwai@...e.com,
        robh+dt@...nel.org, agross@...nel.org,
        linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org,
        alsa-devel@...a-project.org, devicetree@...r.kernel.org,
        linux-usb@...r.kernel.org, quic_jackp@...cinc.com,
        quic_plai@...cinc.com
Subject: Re: [RFC PATCH 06/14] usb: core: hcd: Introduce USB HCD APIs for
 interrupter management

On Fri, Dec 23, 2022 at 03:31:52PM -0800, Wesley Cheng wrote:
> For USB HCDs that can support multiple USB interrupters, expose functions
> that class drivers can utilize for setting up secondary interrupters.
> Class drivers can pass this information to its respective clients, i.e.
> a dedicated DSP.

Where is the locking here that seems to be required when a hcd is
removed from the system and you have data in flight?  What am I missing
here in the design of this?

And yes, HCDs get removed all the time, and will happen more and more in
the future with the design of more systems moving to Thunderbolt/PCIe
designs due to the simplicity of it.

> +/**
> + * usb_set_interruper - Reserve an interrupter

Where is an "interrupter" defined?  I don't know what this term means
sorry, is this in the USB specification somewhere?


> + * @udev: usb device which requested the interrupter
> + * @intr_num: interrupter number to reserve
> + * @dma: iova address to event ring
> + *
> + * Request for a specific interrupter to be reserved for a USB class driver.
> + * This will return the base address to the event ring that was allocated to
> + * the specific interrupter.
> + **/
> +phys_addr_t usb_set_interruper(struct usb_device *udev, int intr_num,
> +							dma_addr_t *dma)
> +{
> +	struct usb_hcd *hcd;
> +	phys_addr_t pa = 0;
> +
> +	hcd = bus_to_hcd(udev->bus);
> +	if (hcd->driver->update_interrupter)
> +		pa = hcd->driver->update_interrupter(hcd, intr_num, dma);
> +
> +	return pa;

Wait, you return a physical address?  What are you going to do with
that?  And what guarantees that the address is valid after you return it
(again, remember memory and devices can be removed at any point in time.

thanks,

greg k-h