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: <CAMZ6RqLnfg=UG_Pisa9M0zYkWEvScZmGbytWmAQPVXLeacRffw@mail.gmail.com>
Date:   Mon, 28 Nov 2022 23:29:12 +0900
From:   Vincent MAILHOL <mailhol.vincent@...adoo.fr>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     Alan Stern <stern@...land.harvard.edu>, linux-can@...r.kernel.org,
        Marc Kleine-Budde <mkl@...gutronix.de>,
        linux-kernel@...r.kernel.org,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        netdev@...r.kernel.org, linux-usb@...r.kernel.org,
        Saeed Mahameed <saeed@...nel.org>,
        Jiri Pirko <jiri@...dia.com>,
        Lukas Magel <lukas.magel@...teo.net>
Subject: Re: [PATCH v4 2/6] can: etas_es58x: add devlink support

On Mon. 28 Nov. 2022 at 22:45, Andrew Lunn <andrew@...n.ch> wrote:
> > > But if a driver does make the call, it should be careful to ensure that
> > > the call happens _after_ the driver is finished using the interface-data
> > > pointer.  For example, after all outstanding URBs have completed, if the
> > > completion handlers will need to call usb_get_intfdata().
> >
> > ACK. I understand that it should be called *after* the completion of
> > any ongoing task.
>
> What sometimes gets people is /sys, /proc. etc. A process can have
> such a file open when the device is unplugged. If the read needs to
> make use of your private data structure, you need to guarantee it
> still exists.  Ideally the core needs to wait and not call the
> disconnect until all such files are closed. Probably the USB core
> does, it is such an obvious issue, but i have no knowledge of USB.

For USB drivers, the parallel of what you are describing are the URBs
(USB request Buffers). The URB are sent asynchronously to the device.
Each URB has a completion handler:
  https://elixir.bootlin.com/linux/v6.0/source/include/linux/usb.h#L1443
It is important to wait for all outstanding URB to complete before
releasing your resources. But once you are able to guarantee that any
ongoing actions were completed, the order in which you kfree() or
usb_set_intfdata() to NULL matters less.

Of course, the USB drivers could also have some /sys/ or /proc/ files
opened, but this is not the case of the etas_es58x. By the way, the
handling of outstanding URBs is done by es58x_free_urbs():
  https://elixir.bootlin.com/linux/v6.0/source/drivers/net/can/usb/etas_es58x/es58x_core.c#L1745
which is called just before the devlink_free() and the usb_set_intfdata().

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ