| 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
| ||
|
Message-id: <55672B5B.2080606@samsung.com>
Date: Thu, 28 May 2015 16:51:07 +0200
From: Krzysztof Opasiak <k.opasiak@...sung.com>
To: Pali Rohár <pali.rohar@...il.com>
Cc: Felipe Balbi <balbi@...com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
Pavel Machek <pavel@....cz>,
Sebastian Reichel <sre@...nel.org>,
Aaro Koskinen <aaro.koskinen@....fi>,
Ivaylo Dimitrov <ivo.g.dimitrov.75@...il.com>
Subject: Re: [PATCH] usb: gadget: nokia: Add mass storage driver to g_nokia
On 05/28/2015 04:31 PM, Pali Rohár wrote:
> On Thursday 28 May 2015 16:27:44 Krzysztof Opasiak wrote:
>>
>>
>> On 05/28/2015 09:47 AM, Pali Rohár wrote:
>>> On Saturday 31 January 2015 10:53:30 Pali Rohár wrote:
>>>> This patch adds removable mass storage support to g_nokia gadget (for N900).
>>>> It means that at runtime block device can be exported or unexported.
>>>> So it does not export anything by default and thus allows to use MyDocs
>>>> partition as before...
>>>>
>>>> Signed-off-by: Pali Rohár <pali.rohar@...il.com>
>>>> ---
>>>> drivers/usb/gadget/legacy/Kconfig | 1 +
>>>> drivers/usb/gadget/legacy/nokia.c | 102 ++++++++++++++++++++++++++++++++++++-
>>>> 2 files changed, 102 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/usb/gadget/legacy/Kconfig b/drivers/usb/gadget/legacy/Kconfig
>>>> index fd48ef3..36f6ba4 100644
>>>> --- a/drivers/usb/gadget/legacy/Kconfig
>>>> +++ b/drivers/usb/gadget/legacy/Kconfig
>>>> @@ -345,6 +345,7 @@ config USB_G_NOKIA
>>>> select USB_F_OBEX
>>>> select USB_F_PHONET
>>>> select USB_F_ECM
>>>> + select USB_F_MASS_STORAGE
>>>> help
>>>> The Nokia composite gadget provides support for acm, obex
>>>> and phonet in only one composite gadget driver.
>>>> diff --git a/drivers/usb/gadget/legacy/nokia.c b/drivers/usb/gadget/legacy/nokia.c
>>>> index 9b8fd70..a09bb50 100644
>>>> --- a/drivers/usb/gadget/legacy/nokia.c
>>>> +++ b/drivers/usb/gadget/legacy/nokia.c
>>>> @@ -24,6 +24,7 @@
>>>> #include "u_phonet.h"
>>>> #include "u_ecm.h"
>>>> #include "gadget_chips.h"
>>>> +#include "f_mass_storage.h"
>>>>
>>>> /* Defines */
>>>>
>>>> @@ -34,6 +35,29 @@ USB_GADGET_COMPOSITE_OPTIONS();
>>>>
>>>> USB_ETHERNET_MODULE_PARAMETERS();
>>>>
>>>> +static struct fsg_module_parameters fsg_mod_data = {
>>>> + .stall = 0,
>>>> + .luns = 2,
>>>> + .removable_count = 2,
>>>> + .removable = { 1, 1, },
>>>> +};
>>>> +
>>>> +FSG_MODULE_PARAMETERS(/* no prefix */, fsg_mod_data);
>>>> +
>>>> +#ifdef CONFIG_USB_GADGET_DEBUG_FILES
>>>> +
>>>> +static unsigned int fsg_num_buffers = CONFIG_USB_GADGET_STORAGE_NUM_BUFFERS;
>>>> +
>>>> +#else
>>>> +
>>>> +/*
>>>> + * Number of buffers we will use.
>>>> + * 2 is usually enough for good buffering pipeline
>>>> + */
>>>> +#define fsg_num_buffers CONFIG_USB_GADGET_STORAGE_NUM_BUFFERS
>>>> +
>>>> +#endif /* CONFIG_USB_DEBUG */
>>>> +
>>>> #define NOKIA_VENDOR_ID 0x0421 /* Nokia */
>>>> #define NOKIA_PRODUCT_ID 0x01c8 /* Nokia Gadget */
>>>>
>>>> @@ -94,6 +118,8 @@ static struct usb_function *f_obex1_cfg2;
>>>> static struct usb_function *f_obex2_cfg2;
>>>> static struct usb_function *f_phonet_cfg1;
>>>> static struct usb_function *f_phonet_cfg2;
>>>> +static struct usb_function *f_msg_cfg1;
>>>> +static struct usb_function *f_msg_cfg2;
>>>>
>>>>
>>>> static struct usb_configuration nokia_config_500ma_driver = {
>>>> @@ -117,6 +143,7 @@ static struct usb_function_instance *fi_ecm;
>>>> static struct usb_function_instance *fi_obex1;
>>>> static struct usb_function_instance *fi_obex2;
>>>> static struct usb_function_instance *fi_phonet;
>>>> +static struct usb_function_instance *fi_msg;
>>>>
>>>> static int __init nokia_bind_config(struct usb_configuration *c)
>>>> {
>>>> @@ -125,6 +152,8 @@ static int __init nokia_bind_config(struct usb_configuration *c)
>>>> struct usb_function *f_obex1 = NULL;
>>>> struct usb_function *f_ecm;
>>>> struct usb_function *f_obex2 = NULL;
>>>> + struct usb_function *f_msg;
>>>> + struct fsg_opts *fsg_opts;
>>>> int status = 0;
>>>> int obex1_stat = -1;
>>>> int obex2_stat = -1;
>>>> @@ -160,6 +189,12 @@ static int __init nokia_bind_config(struct usb_configuration *c)
>>>> goto err_get_ecm;
>>>> }
>>>>
>>>> + f_msg = usb_get_function(fi_msg);
>>>> + if (IS_ERR(f_msg)) {
>>>> + status = PTR_ERR(f_msg);
>>>> + goto err_get_msg;
>>>> + }
>>>> +
>>>> if (!IS_ERR_OR_NULL(f_phonet)) {
>>>> phonet_stat = usb_add_function(c, f_phonet);
>>>> if (phonet_stat)
>>>> @@ -187,21 +222,36 @@ static int __init nokia_bind_config(struct usb_configuration *c)
>>>> pr_debug("could not bind ecm config %d\n", status);
>>>> goto err_ecm;
>>>> }
>>>> +
>>>> + fsg_opts = fsg_opts_from_func_inst(fi_msg);
>>>> +
>>>> + status = fsg_common_run_thread(fsg_opts->common);
>>>> + if (status)
>>>> + goto err_msg;
>>>> +
>>>> + status = usb_add_function(c, f_msg);
>>>> + if (status)
>>>> + goto err_msg;
>>>> +
>>>> if (c == &nokia_config_500ma_driver) {
>>>> f_acm_cfg1 = f_acm;
>>>> f_ecm_cfg1 = f_ecm;
>>>> f_phonet_cfg1 = f_phonet;
>>>> f_obex1_cfg1 = f_obex1;
>>>> f_obex2_cfg1 = f_obex2;
>>>> + f_msg_cfg1 = f_msg;
>>>> } else {
>>>> f_acm_cfg2 = f_acm;
>>>> f_ecm_cfg2 = f_ecm;
>>>> f_phonet_cfg2 = f_phonet;
>>>> f_obex1_cfg2 = f_obex1;
>>>> f_obex2_cfg2 = f_obex2;
>>>> + f_msg_cfg2 = f_msg;
>>>> }
>>>>
>>>> return status;
>>>> +err_msg:
>>>> + usb_remove_function(c, f_ecm);
>>>> err_ecm:
>>>> usb_remove_function(c, f_acm);
>>>> err_conf:
>>>> @@ -211,6 +261,8 @@ err_conf:
>>>> usb_remove_function(c, f_obex1);
>>>> if (!phonet_stat)
>>>> usb_remove_function(c, f_phonet);
>>>> + usb_put_function(f_msg);
>>>> +err_get_msg:
>>>> usb_put_function(f_ecm);
>>>> err_get_ecm:
>>>> usb_put_function(f_acm);
>>>> @@ -227,6 +279,8 @@ err_get_acm:
>>>> static int __init nokia_bind(struct usb_composite_dev *cdev)
>>>> {
>>>> struct usb_gadget *gadget = cdev->gadget;
>>>> + struct fsg_opts *fsg_opts;
>>>> + struct fsg_config fsg_config;
>>>> int status;
>>>>
>>>> status = usb_string_ids_tab(cdev, strings_dev);
>>>> @@ -267,11 +321,46 @@ static int __init nokia_bind(struct usb_composite_dev *cdev)
>>>> goto err_acm_inst;
>>>> }
>>>>
>>>> + fi_msg = usb_get_function_instance("mass_storage");
>>>> + if (IS_ERR(fi_msg)) {
>>>> + status = PTR_ERR(fi_msg);
>>>> + goto err_ecm_inst;
>>>> + }
>>>> +
>>>> + /* set up mass storage function */
>>>> + fsg_config_from_params(&fsg_config, &fsg_mod_data, fsg_num_buffers);
>>>> + fsg_config.vendor_name = "Nokia";
>>>> + fsg_config.product_name = "N900";
>>>> +
>>>> + fsg_opts = fsg_opts_from_func_inst(fi_msg);
>>>> + fsg_opts->no_configfs = true;
>>>> +
>>>> + status = fsg_common_set_num_buffers(fsg_opts->common, fsg_num_buffers);
>>>> + if (status)
>>>> + goto err_msg_inst;
>>>> +
>>>> + status = fsg_common_set_nluns(fsg_opts->common, fsg_config.nluns);
>>>> + if (status)
>>>> + goto err_msg_buf;
>>>> +
>>>> + status = fsg_common_set_cdev(fsg_opts->common, cdev, fsg_config.can_stall);
>>>> + if (status)
>>>> + goto err_msg_set_nluns;
>>>> +
>>>> + fsg_common_set_sysfs(fsg_opts->common, true);
>>>> +
>>>> + status = fsg_common_create_luns(fsg_opts->common, &fsg_config);
>>>> + if (status)
>>>> + goto err_msg_set_nluns;
>>>> +
>>>> + fsg_common_set_inquiry_string(fsg_opts->common, fsg_config.vendor_name,
>>>> + fsg_config.product_name);
>>>> +
>>>> /* finally register the configuration */
>>>> status = usb_add_config(cdev, &nokia_config_500ma_driver,
>>>> nokia_bind_config);
>>>> if (status < 0)
>>>> - goto err_ecm_inst;
>>>> + goto err_msg_set_cdev;
>>>>
>>>> status = usb_add_config(cdev, &nokia_config_100ma_driver,
>>>> nokia_bind_config);
>>>> @@ -292,6 +381,14 @@ err_put_cfg1:
>>>> if (!IS_ERR_OR_NULL(f_phonet_cfg1))
>>>> usb_put_function(f_phonet_cfg1);
>>>> usb_put_function(f_ecm_cfg1);
>>>> +err_msg_set_cdev:
>>>> + fsg_common_remove_luns(fsg_opts->common);
>>>> +err_msg_set_nluns:
>>>> + fsg_common_free_luns(fsg_opts->common);
>>>> +err_msg_buf:
>>>> + fsg_common_free_buffers(fsg_opts->common);
>>>> +err_msg_inst:
>>>> + usb_put_function_instance(fi_msg);
>>>> err_ecm_inst:
>>>> usb_put_function_instance(fi_ecm);
>>>> err_acm_inst:
>>>> @@ -325,7 +422,10 @@ static int __exit nokia_unbind(struct usb_composite_dev *cdev)
>>>> usb_put_function(f_acm_cfg2);
>>>> usb_put_function(f_ecm_cfg1);
>>>> usb_put_function(f_ecm_cfg2);
>>>> + usb_put_function(f_msg_cfg1);
>>>> + usb_put_function(f_msg_cfg2);
>>>>
>>>> + usb_put_function_instance(fi_msg);
>>>> usb_put_function_instance(fi_ecm);
>>>> if (!IS_ERR(fi_obex2))
>>>> usb_put_function_instance(fi_obex2);
>>>
>>> Opening discussion about this patch again as this is still only one
>>> solution how to use use mass storage without breaking other stuff...
>>>
>>> Please understand finally that usb networking is very very important for
>>> development on this device and usb mass storage is needed for
>>> transferring bigger data.
>>>
>>> Also to clarify potential regressions: This patch adds *optional* usb
>>> mass storage support and block device can be attached/detached to driver
>>> at runtime. It does *not* enforce to export some (mmc) device
>>> automatically. It is optional and userspace can decide...
>>>
>>> So MyDocs N900 partition is not automatically enforced to be exported
>>> via usb (as some people thought) and so it does not break usb networking
>>> or mass storage mode or MyDocs parition on N900 with Maemo system.
>>>
>>
>> Instead of adding mass storage to legacy gadget you may switch to
>> configfs interface and compose equivalent of g_nokia + mass storage
>> without any changes in kernel.
>>
>> Best regards,
>>
>
> That requires userspace. And there are developers who use nfsroot via
> usb networking and so userspace will be there after properly
> initializing usb gadget... So it is not easy and reason why I'm opening
> this discussion again.
>
Couldn't you simply use initramfs to compose your gadget?
--
Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists