On Tue, Jul 08 2014, Andrzej Pietrasiewicz <andrzej.p@xxxxxxxxxxx> wrote:
> Add support for OS descriptors. The new format of descriptors is used,
> because the "flags" field is required for extensions. os_count gives
> the number of OSDesc[] elements.
> The format of descriptors is given in include/uapi/linux/usb/functionfs.h.
>
> For extended properties descriptor the usb_ext_prop_desc structure covers
> only a part of a descriptor, because the wPropertyNameLength is unknown
> up front.
>
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@xxxxxxxxxxx>
Acked-by: Michal Nazarewicz <mina86@xxxxxxxxxx>
Just a few minor comments.
> @@ -1856,11 +1865,189 @@ static int __ffs_data_do_entity(enum ffs_entity_type type,
> return 0;
> }
>
> +static int __ffs_do_os_desc_header(enum ffs_os_desc_type *next_type,
> + struct usb_os_desc_header *desc)
> +{
> + u16 bcd_version = le16_to_cpu(desc->bcdVersion);
> + u16 w_index = le16_to_cpu(desc->wIndex);
> +
> + if (bcd_version != 1) {
> + pr_vdebug("unsupported os descriptors version: %d",
> + bcd_version);
> + return -EINVAL;
> + }
> + switch (w_index) {
> + case 0x4:
> + *next_type = FFS_OS_DESC_EXT_COMPAT;
> + break;
> + case 0x5:
> + *next_type = FFS_OS_DESC_EXT_PROP;
> + break;
> + default:
> + pr_vdebug("unsupported os descriptor type: %d", w_index);
> + return -EINVAL;
> + }
> +
> + return sizeof(*desc);
> +}
> +
> +/*
> + * Process all extended compatibility/extended property descriptors
> + * of a feature descriptor
> + */
> +static int __must_check ffs_do_single_os_desc(char *data, unsigned len,
> + enum ffs_os_desc_type type,
> + u16 feature_count,
> + ffs_os_desc_callback entity,
> + void *priv,
> + struct usb_os_desc_header *h)
> +{
> + int ret;
> + const unsigned _len = len;
> +
> + ENTER();
> +
> + /* loop over all ext compat/ext prop descriptors */
> + while (feature_count--) {
> + ret = entity(type, h, data, len, priv);
> + if (unlikely(ret < 0)) {
> + pr_debug("bad OS descriptor, type: %d\n", type);
> + return ret;
> + }
> + data += ret;
> + len -= ret;
> + }
> + return _len - len;
> +}
> +
> +/* Process a number of complete Feature Descriptors (Ext Compat or Ext Prop) */
> +static int __must_check ffs_do_os_descs(unsigned count,
> + char *data, unsigned len,
> + ffs_os_desc_callback entity, void *priv)
> +{
> + const unsigned _len = len;
> + unsigned long num = 0;
> +
> + ENTER();
> +
> + for (num = 0; num < count; ++num) {
> + int ret;
> + enum ffs_os_desc_type type;
> + u16 feature_count;
> + struct usb_os_desc_header *desc = (void *)data;
> +
> + if (len < sizeof(*desc))
> + return -EINVAL;
> +
> + /*
> + * Record "descriptor" entity.
> + * Process dwLength, bcdVersion, wIndex, get b/wCount.
> + * Move the data pointer to the beginning of extended
> + * compatibilities proper or extended properties proper
> + * portions of the data
> + */
So dwLength is never accessed at all. It probably should? At least:
if (le32_to_cpu(desc->dwLength) > len)
return -EINVAL;
> + ret = __ffs_do_os_desc_header(&type, desc);
> + if (unlikely(ret < 0)) {
> + pr_debug("entity OS_DESCRIPTOR(%02lx); ret = %d\n",
> + num, ret);
> + return ret;
> + }
> + /*
> + * 16-bit hex "?? 00" Little Endian looks like 8-bit hex "??"
> + */
> + feature_count = le16_to_cpu(desc->wCount);
> + if (type == FFS_OS_DESC_EXT_COMPAT &&
> + (feature_count > 255 || desc->Reserved))
> + return -EINVAL;
> + len -= ret;
> + data += ret;
> +
> + /*
> + * Process all function/property descriptors
> + * of this Feature Descriptor
> + */
> + ret = ffs_do_single_os_desc(data, len, type,
> + feature_count, entity, priv, desc);
> + if (unlikely(ret < 0)) {
> + pr_debug("%s returns %d\n", __func__, ret);
> + return ret;
> + }
> +
> + len -= ret;
> + data += ret;
> + }
> + return _len - len;
> +}
> +
> +/**
> + * Validate contents of the buffer from userspace related to OS descriptors.
> + */
> +static int __ffs_data_do_os_desc(enum ffs_os_desc_type type,
> + struct usb_os_desc_header *h, void *data,
> + unsigned len, void *priv)
> +{
> + struct ffs_data *ffs = priv;
> + u8 length;
> +
> + ENTER();
> +
> + switch (type) {
> + case FFS_OS_DESC_EXT_COMPAT: {
> + struct usb_ext_compat_desc *d = data;
> + int i;
> +
> + if (d->bFirstInterfaceNumber >= ffs->interfaces_count ||
> + d->Reserved1)
> + return -EINVAL;
> + for (i = 0; i < ARRAY_SIZE(d->Reserved2); ++i)
> + if (d->Reserved2[i])
> + return -EINVAL;
> +
> + if (len < sizeof(*d))
> + return -EINVAL;
This check should be at the beginning of the case.
> + length = sizeof(struct usb_ext_compat_desc);
> + }
> + break;
> + case FFS_OS_DESC_EXT_PROP: {
> + struct usb_ext_prop_desc *d = data;
> + u32 type, pdl;
> + u16 pnl;
> +
> + if (len < sizeof(*d) || h->interface >= ffs->interfaces_count)
> + return -EINVAL;
> + length = le32_to_cpu(d->dwSize);
> + type = le32_to_cpu(d->dwPropertyDataType);
> + if (type < USB_EXT_PROP_UNICODE ||
> + type > USB_EXT_PROP_UNICODE_MULTI) {
> + pr_vdebug("unsupported os descriptor property type: %d",
> + type);
> + return -EINVAL;
> + }
> + pnl = le16_to_cpu(d->wPropertyNameLength);
> + pdl = le32_to_cpu(*(u32 *)((u8 *)data + 10 + pnl));
> + if (length != 14 + pnl + pdl) {
> + pr_vdebug("invalid os descriptor length: %d pnl:%d pdl:%d (descriptor %d)\n",
> + length, pnl, pdl, type);
> + return -EINVAL;
> + }
> + ++ffs->ms_os_descs_ext_prop_count;
> + /* property name reported to the host as "WCHAR"s */
> + ffs->ms_os_descs_ext_prop_name_len += pnl * 2;
> + ffs->ms_os_descs_ext_prop_data_len += pdl;
> + }
> + break;
> + default:
> + pr_vdebug("unknown descriptor: %d\n", type);
> + return -EINVAL;
> + }
> + return length;
> +}
> +
> static int __ffs_data_got_descs(struct ffs_data *ffs,
> char *const _data, size_t len)
> {
> char *data = _data, *raw_descs;
> - unsigned counts[3], flags;
> + unsigned os_descs_count = 0, counts[3], flags;
> int ret = -EINVAL, i;
>
> ENTER();
> @@ -2267,6 +2469,86 @@ static int __ffs_func_bind_do_nums(enum ffs_entity_type type, u8 *valuep,
> return 0;
> }
>
> +static int __ffs_func_bind_do_os_desc(enum ffs_os_desc_type type,
> + struct usb_os_desc_header *h, void *data,
> + unsigned len, void *priv)
> +{
> + struct ffs_function *func = priv;
> + u8 length = 0;
> +
> + switch (type) {
> + case FFS_OS_DESC_EXT_COMPAT: {
> + struct usb_ext_compat_desc *desc = data;
> + struct usb_os_desc_table *t;
> +
> + t = &func->function.os_desc_table[desc->bFirstInterfaceNumber];
> + t->if_id = func->interfaces_nums[desc->bFirstInterfaceNumber];
> + /* Copy CompatibleID and SubCompatibleID in one go, hence 16. */
So now there's no “16” in the code, so the comment is confusing. ;)
> + memcpy(t->os_desc->ext_compat_id, &desc->CompatibleID,
> + ARRAY_SIZE(desc->CompatibleID) +
> + ARRAY_SIZE(desc->SubCompatibleID));
> + length = sizeof(*desc);
> + }
> + break;
> + case FFS_OS_DESC_EXT_PROP: {
> + struct usb_ext_prop_desc *desc = data;
> + struct usb_os_desc_table *t;
> + struct usb_os_desc_ext_prop *ext_prop;
> + char *ext_prop_name;
> + char *ext_prop_data;
> +
> + t = &func->function.os_desc_table[h->interface];
> + t->if_id = func->interfaces_nums[h->interface];
> +
> + ext_prop = func->ffs->ms_os_descs_ext_prop_avail;
> + func->ffs->ms_os_descs_ext_prop_avail += sizeof(*ext_prop);
> +
> + ext_prop->type = le32_to_cpu(desc->dwPropertyDataType);
> + ext_prop->name_len = le16_to_cpu(desc->wPropertyNameLength);
> + ext_prop->data_len = le32_to_cpu(*(u32 *)
> + usb_ext_prop_data_len_ptr(data, ext_prop->name_len));
> + length = ext_prop->name_len + ext_prop->data_len + 14;
> +
> + ext_prop_name = func->ffs->ms_os_descs_ext_prop_name_avail;
> + func->ffs->ms_os_descs_ext_prop_name_avail +=
> + ext_prop->name_len;
> +
> + ext_prop_data = func->ffs->ms_os_descs_ext_prop_data_avail;
> + func->ffs->ms_os_descs_ext_prop_data_avail +=
> + ext_prop->data_len;
> + memcpy(ext_prop_data,
> + usb_ext_prop_data_ptr(data, ext_prop->name_len),
> + ext_prop->data_len);
> + /* unicode data reported to the host as "WCHAR"s */
> + switch (ext_prop->type) {
> + case USB_EXT_PROP_UNICODE:
> + case USB_EXT_PROP_UNICODE_ENV:
> + case USB_EXT_PROP_UNICODE_LINK:
> + case USB_EXT_PROP_UNICODE_MULTI:
> + ext_prop->data_len *= 2;
> + break;
> + }
> + ext_prop->data = ext_prop_data;
> +
> + memcpy(ext_prop_name, usb_ext_prop_name_ptr(data),
> + ext_prop->name_len);
> + /* property name reported to the host as "WCHAR"s */
> + ext_prop->name_len *= 2;
> + ext_prop->name = ext_prop_name;
> +
> + t->os_desc->ext_prop_len +=
> + ext_prop->name_len + ext_prop->data_len + 14;
> + ++t->os_desc->ext_prop_count;
> + list_add_tail(&ext_prop->entry, &t->os_desc->ext_prop);
> + }
> + break;
> + default:
> + pr_vdebug("unknown descriptor: %d\n", type);
> + }
> +
> + return length;
> +}
> +
> static inline struct f_fs_opts *ffs_do_functionfs_bind(struct usb_function *f,
> struct usb_configuration *c)
> {
--
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
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
