On Mon, Jun 22 2015, Krzysztof Opasiak wrote:
> GET_MAX_LUN request should return number of realy created LUNs
> not the size of preallocated array.
>
> This patch changes fsg_common_set_nluns() to fsg_common_set_max_luns()
> which now only allocates an empty array for luns and set nluns
> to 0. While LUNS are create we simply count them and store result
> in nluns which is send later to host.
>
> Reported-by: David Fisher <david.fisher1@xxxxxxxxxxxx>
> Signed-off-by: Krzysztof Opasiak <k.opasiak@xxxxxxxxxxx>
At this point I would just change common->luns to be an array rather
than a pointer. This would remove need for max_luns all together.
> ---
> drivers/usb/gadget/function/f_mass_storage.c | 69 ++++++++++++++------------
> drivers/usb/gadget/function/f_mass_storage.h | 2 +-
> drivers/usb/gadget/legacy/acm_ms.c | 6 +--
> drivers/usb/gadget/legacy/mass_storage.c | 6 +--
> drivers/usb/gadget/legacy/multi.c | 6 +--
> 5 files changed, 48 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c
> index 4257238..7e37070 100644
> --- a/drivers/usb/gadget/function/f_mass_storage.c
> +++ b/drivers/usb/gadget/function/f_mass_storage.c
> @@ -280,6 +280,7 @@ struct fsg_common {
> u8 cmnd[MAX_COMMAND_SIZE];
>
> unsigned int nluns;
> + unsigned int max_luns;
> unsigned int lun;
> struct fsg_lun **luns;
> struct fsg_lun *curlun;
> @@ -2131,7 +2132,7 @@ static int received_cbw(struct fsg_dev *fsg, struct fsg_buffhd *bh)
> }
>
> /* Is the CBW meaningful? */
> - if (cbw->Lun >= FSG_MAX_LUNS || cbw->Flags & ~US_BULK_FLAG_IN ||
> + if (cbw->Lun >= common->nluns || cbw->Flags & ~US_BULK_FLAG_IN ||
> cbw->Length <= 0 || cbw->Length > MAX_COMMAND_SIZE) {
> DBG(fsg, "non-meaningful CBW: lun = %u, flags = 0x%x, "
> "cmdlen %u\n",
> @@ -2411,8 +2412,7 @@ static void handle_exception(struct fsg_common *common)
> else {
> for (i = 0; i < common->nluns; ++i) {
> curlun = common->luns[i];
> - if (!curlun)
> - continue;
> + WARN_ON(!curlun);
> curlun->prevent_medium_removal = 0;
> curlun->sense_data = SS_NO_SENSE;
> curlun->unit_attention_data = SS_NO_SENSE;
> @@ -2558,7 +2558,9 @@ static int fsg_main_thread(void *common_)
> down_write(&common->filesem);
> for (; i--; ++curlun_it) {
> struct fsg_lun *curlun = *curlun_it;
> - if (!curlun || !fsg_lun_is_open(curlun))
> +
> + WARN_ON(!curlun);
> + if (!fsg_lun_is_open(curlun))
> continue;
>
> fsg_lun_close(curlun);
> @@ -2759,6 +2761,8 @@ static void _fsg_common_remove_luns(struct fsg_common *common, int n)
> if (common->luns[i]) {
> fsg_common_remove_lun(common->luns[i], common->sysfs);
> common->luns[i] = NULL;
> + WARN_ON(common->nluns == 0);
> + common->nluns--;
> }
> }
>
> @@ -2773,20 +2777,22 @@ void fsg_common_free_luns(struct fsg_common *common)
> fsg_common_remove_luns(common);
> kfree(common->luns);
> common->luns = NULL;
> + common->max_luns = 0;
> + common->nluns = 0;
> }
> EXPORT_SYMBOL_GPL(fsg_common_free_luns);
>
> -int fsg_common_set_nluns(struct fsg_common *common, int nluns)
> +int fsg_common_set_max_luns(struct fsg_common *common, int max_luns)
> {
> struct fsg_lun **curlun;
>
> /* Find out how many LUNs there should be */
> - if (nluns < 1 || nluns > FSG_MAX_LUNS) {
> - pr_err("invalid number of LUNs: %u\n", nluns);
> + if (max_luns < 1 || max_luns > FSG_MAX_LUNS) {
> + pr_err("invalid number of LUNs: %u\n", max_luns);
> return -EINVAL;
> }
>
> - curlun = kcalloc(nluns, sizeof(*curlun), GFP_KERNEL);
> + curlun = kcalloc(max_luns, sizeof(*curlun), GFP_KERNEL);
> if (unlikely(!curlun))
> return -ENOMEM;
>
> @@ -2794,13 +2800,15 @@ int fsg_common_set_nluns(struct fsg_common *common, int nluns)
> fsg_common_free_luns(common);
>
> common->luns = curlun;
> - common->nluns = nluns;
> + common->max_luns = max_luns;
> + common->nluns = 0;
>
> - pr_info("Number of LUNs=%d\n", common->nluns);
> + pr_info("Number of LUNs=%d, max number of LUNs=%d\n",
> + common->nluns, common->max_luns);
>
> return 0;
> }
> -EXPORT_SYMBOL_GPL(fsg_common_set_nluns);
> +EXPORT_SYMBOL_GPL(fsg_common_set_max_luns);
>
> void fsg_common_set_ops(struct fsg_common *common,
> const struct fsg_operations *ops)
> @@ -2882,7 +2890,7 @@ int fsg_common_create_lun(struct fsg_common *common, struct fsg_lun_config *cfg,
> char *pathbuf, *p;
> int rc = -ENOMEM;
>
> - if (!common->nluns || !common->luns)
> + if (!common->max_luns || !common->luns)
> return -ENODEV;
>
> if (common->luns[id])
> @@ -2924,6 +2932,7 @@ int fsg_common_create_lun(struct fsg_common *common, struct fsg_lun_config *cfg,
> }
>
> common->luns[id] = lun;
> + common->nluns++;
>
> if (cfg->filename) {
> rc = fsg_lun_open(lun, cfg->filename);
> @@ -2955,6 +2964,7 @@ error_lun:
> device_unregister(&lun->dev);
> fsg_lun_close(lun);
> common->luns[id] = NULL;
> + common->nluns--;
> error_sysfs:
> kfree(lun);
> return rc;
> @@ -2966,7 +2976,10 @@ int fsg_common_create_luns(struct fsg_common *common, struct fsg_config *cfg)
> char buf[8]; /* enough for 100000000 different numbers, decimal */
> int i, rc;
>
> - for (i = 0; i < common->nluns; ++i) {
> + if (cfg->nluns > common->max_luns)
> + return -EINVAL;
> +
> + for (i = 0; i < cfg->nluns; ++i) {
> snprintf(buf, sizeof(buf), "lun%d", i);
> rc = fsg_common_create_lun(common, &cfg->luns[i], i, buf, NULL);
> if (rc)
> @@ -3031,7 +3044,7 @@ static void fsg_common_release(struct kref *ref)
>
> if (likely(common->luns)) {
> struct fsg_lun **lun_it = common->luns;
> - unsigned i = common->nluns;
> + unsigned i = common->max_luns;
>
> /* In error recovery common->nluns may be zero. */
> for (; i; --i, ++lun_it) {
> @@ -3058,6 +3071,7 @@ static void fsg_common_release(struct kref *ref)
> static int fsg_bind(struct usb_configuration *c, struct usb_function *f)
> {
> struct fsg_dev *fsg = fsg_from_func(f);
> + struct fsg_common *common = fsg->common;
> struct usb_gadget *gadget = c->cdev->gadget;
> int i;
> struct usb_ep *ep;
> @@ -3070,25 +3084,16 @@ static int fsg_bind(struct usb_configuration *c, struct usb_function *f)
> * or LUNs ids are not contiguous.
> */
> if (likely(common->luns)) {
> - bool found_null = false;
> -
> - for (i = 0; i < common->nluns; ++i) {
> - if (!common->luns[i]) {
> - found_null = true;
> - continue;
> - }
> -
> - if (!found_null) {
> - continue;
> - } else {
> - pr_err("LUN ids should be contiguous.\n");
> - return -EINVAL;
> - }
> - }
> + for (i = 0; i < common->max_luns; ++i)
> + if (!common->luns[i])
> + break;
>
> - if (i == 0 || !common->luns[i]) {
> + if (i == 0) {
> pr_err("There should be at least one LUN.\n");
> return -EINVAL;
> + } else if (i < common->nluns) {
> + pr_err("LUN ids should be contiguous.\n");
> + return -EINVAL;
> }
> } else {
> return -EINVAL;
> @@ -3388,6 +3393,8 @@ static void fsg_lun_drop(struct config_group *group, struct config_item *item)
>
> fsg_common_remove_lun(lun_opts->lun, fsg_opts->common->sysfs);
> fsg_opts->common->luns[lun_opts->lun_id] = NULL;
> + WARN_ON(fsg_opts->common->nluns == 0);
> + fsg_opts->common->nluns--;
> lun_opts->lun_id = 0;
> mutex_unlock(&fsg_opts->lock);
>
> @@ -3540,7 +3547,7 @@ static struct usb_function_instance *fsg_alloc_inst(void)
> rc = PTR_ERR(opts->common);
> goto release_opts;
> }
> - rc = fsg_common_set_nluns(opts->common, FSG_MAX_LUNS);
> + rc = fsg_common_set_max_luns(opts->common, FSG_MAX_LUNS);
> if (rc)
> goto release_opts;
>
> diff --git a/drivers/usb/gadget/function/f_mass_storage.h b/drivers/usb/gadget/function/f_mass_storage.h
> index b4866fc..47d366a 100644
> --- a/drivers/usb/gadget/function/f_mass_storage.h
> +++ b/drivers/usb/gadget/function/f_mass_storage.h
> @@ -143,7 +143,7 @@ void fsg_common_remove_luns(struct fsg_common *common);
>
> void fsg_common_free_luns(struct fsg_common *common);
>
> -int fsg_common_set_nluns(struct fsg_common *common, int nluns);
> +int fsg_common_set_max_luns(struct fsg_common *common, int max_luns);
>
> void fsg_common_set_ops(struct fsg_common *common,
> const struct fsg_operations *ops);
> diff --git a/drivers/usb/gadget/legacy/acm_ms.c b/drivers/usb/gadget/legacy/acm_ms.c
> index 1194b09..1c56196 100644
> --- a/drivers/usb/gadget/legacy/acm_ms.c
> +++ b/drivers/usb/gadget/legacy/acm_ms.c
> @@ -200,9 +200,9 @@ static int acm_ms_bind(struct usb_composite_dev *cdev)
> if (status)
> goto fail;
>
> - status = fsg_common_set_nluns(opts->common, config.nluns);
> + status = fsg_common_set_max_luns(opts->common, config.nluns);
> if (status)
> - goto fail_set_nluns;
> + goto fail_set_max_luns;
>
> status = fsg_common_set_cdev(opts->common, cdev, config.can_stall);
> if (status)
> @@ -240,7 +240,7 @@ fail_string_ids:
> fsg_common_remove_luns(opts->common);
> fail_set_cdev:
> fsg_common_free_luns(opts->common);
> -fail_set_nluns:
> +fail_set_max_luns:
> fsg_common_free_buffers(opts->common);
> fail:
> usb_put_function_instance(fi_msg);
> diff --git a/drivers/usb/gadget/legacy/mass_storage.c b/drivers/usb/gadget/legacy/mass_storage.c
> index e7bfb08..7c2074c 100644
> --- a/drivers/usb/gadget/legacy/mass_storage.c
> +++ b/drivers/usb/gadget/legacy/mass_storage.c
> @@ -191,9 +191,9 @@ static int msg_bind(struct usb_composite_dev *cdev)
> if (status)
> goto fail;
>
> - status = fsg_common_set_nluns(opts->common, config.nluns);
> + status = fsg_common_set_max_luns(opts->common, config.nluns);
> if (status)
> - goto fail_set_nluns;
> + goto fail_set_max_luns;
>
> fsg_common_set_ops(opts->common, &ops);
>
> @@ -228,7 +228,7 @@ fail_string_ids:
> fsg_common_remove_luns(opts->common);
> fail_set_cdev:
> fsg_common_free_luns(opts->common);
> -fail_set_nluns:
> +fail_set_max_luns:
> fsg_common_free_buffers(opts->common);
> fail:
> usb_put_function_instance(fi_msg);
> diff --git a/drivers/usb/gadget/legacy/multi.c b/drivers/usb/gadget/legacy/multi.c
> index b21b51f..df441b2 100644
> --- a/drivers/usb/gadget/legacy/multi.c
> +++ b/drivers/usb/gadget/legacy/multi.c
> @@ -407,9 +407,9 @@ static int __ref multi_bind(struct usb_composite_dev *cdev)
> if (status)
> goto fail2;
>
> - status = fsg_common_set_nluns(fsg_opts->common, config.nluns);
> + status = fsg_common_set_max_luns(fsg_opts->common, config.nluns);
> if (status)
> - goto fail_set_nluns;
> + goto fail_set_max_luns;
>
> status = fsg_common_set_cdev(fsg_opts->common, cdev, config.can_stall);
> if (status)
> @@ -449,7 +449,7 @@ fail_string_ids:
> fsg_common_remove_luns(fsg_opts->common);
> fail_set_cdev:
> fsg_common_free_luns(fsg_opts->common);
> -fail_set_nluns:
> +fail_set_max_luns:
> fsg_common_free_buffers(fsg_opts->common);
> fail2:
> usb_put_function_instance(fi_msg);
> --
> 1.7.9.5
>
--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michał “mina86” Nazarewicz (o o)
ooo +--<mpn@xxxxxxxxxx>--<xmpp:mina86@xxxxxxxxxx>--ooO--(_)--Ooo--
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
