Re: [PATCH] bsg referencing bus driver module

[James Bottomley]

On Thu, 2018-04-19 at 15:10 -0700, Anatoliy Glagolev wrote:
> Updated: rebased on recent Linux, cc-ed maintainers per instructions
> in MAINTAINERS file
> 
> From df939b80d02bf37b21efaaef8ede86cfd39b0cb8 Mon Sep 17 00:00:00
> 2001
> From: Anatoliy Glagolev <aglagolev@xxxxxxxxxxxxxxx>
> Date: Thu, 19 Apr 2018 15:06:06 -0600
> Subject: [PATCH] bsg referencing parent module
> 
> Fixing a bug when bsg layer holds the last reference to device
> when the device's module has been unloaded. Upon dropping the
> reference the device's release function may touch memory of
> the unloaded module.
> ---
> Âblock/bsg-lib.cÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ| 24 ++++++++++++++++++++++--
> Âblock/bsg.cÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ| 29 ++++++++++++++++++++++++++---
> Âdrivers/scsi/scsi_transport_fc.c |ÂÂ8 ++++++--
> Âinclude/linux/bsg-lib.hÂÂÂÂÂÂÂÂÂÂ|ÂÂ4 ++++
> Âinclude/linux/bsg.hÂÂÂÂÂÂÂÂÂÂÂÂÂÂ|ÂÂ5 +++++
> Â5 files changed, 63 insertions(+), 7 deletions(-)
> 
> diff --git a/block/bsg-lib.c b/block/bsg-lib.c
> index fc2e5ff..90c28fd 100644
> --- a/block/bsg-lib.c
> +++ b/block/bsg-lib.c
> @@ -309,6 +309,25 @@ struct request_queue *bsg_setup_queue(struct
> device *dev, const char *name,
> Â bsg_job_fn *job_fn, int dd_job_size,
> Â void (*release)(struct device *))
> Â{
> + return bsg_setup_queue_ex(dev, name, job_fn, dd_job_size, release,
> + NULL);
> +}
> +EXPORT_SYMBOL_GPL(bsg_setup_queue);
> +
> +/**
> + * bsg_setup_queue - Create and add the bsg hooks so we can receive
> requests
> + * @dev: device to attach bsg device to
> + * @name: device to give bsg device
> + * @job_fn: bsg job handler
> + * @dd_job_size: size of LLD data needed for each job
> + * @release: @dev release function
> + * @dev_module: @dev's module
> + */
> +struct request_queue *bsg_setup_queue_ex(struct device *dev, const
> char *name,
> + bsg_job_fn *job_fn, int dd_job_size,
> + void (*release)(struct device *),
> + struct module *dev_module)

This patch isn't applyable because your mailer has changed all the tabs
to spaces.

I also think there's no need to do it this way.  I think what we need
is for fc_bsg_remove() to wait until the bsg queue is drained.  It does
look like the author thought this happened otherwise the code wouldn't
have the note.  If we fix it that way we can do the same thing in all
the other transport classes that use bsg (which all have a similar
issue).

James