On Fri, 12 Jun 2015, Stefan Koch wrote:
> This is a patch that introduces an interface authorization for USB devices.
> The kernel supports already a device authorization bacause of wireless USB.
>
> But the new interface authorization allows to enable or disable individual
> interfaces per bitmask
> instead allow or deny a whole device.
>
> The patch is made now much simplier as suggested by Alan Stern.
>
> Each patch depends on all patches with a lesser number.
>
> Stefan Koch (5):
> usb: Add usb interface authorization: Declare attributes of structures
> usb: Add usb interface authorization: Introduces the default interface
> authorization
> usb: Add usb interface authorization: Control interface probing and
> claiming
> usb: Add usb interface authorization: Introduces the usb interface
> authorization.
> usb: Add usb interface authorization: SysFS part of usb interface
> authorization.
There is a lot of questionable material here.
First of all, I agree with Krzysztof that having an "authorized"
attribute in each interface's sysfs directory would be simpler and
easier to use than having a bitmask of all authorized interfaces.
Secondly, the patches have not been carefully edited. There are
several misspelled words in comments and descriptions. And why does
patch 3/5 modify drivers/base/base.h and include/linux/device.h?
Thirdly, what is the purpose of the mask_changed bit? The changelog
describes it as "a status bit to control a manual setting of the mask",
which is not very clear. _How_ does it control manual setting of the
mask? _Why_ does manual setting of the mask need to be controlled?
Fourthly, it doesn't look like usb_set_configuration() does the right
thing. It should call usb_enable_interface() whether the interface is
authorized or not.
Fifthly, the code style is awkward in several places. For instance,
it looks silly to do this (usb_alloc_dev in patch 2/5):
dev->mask = 0;
/* invert the mask. each bit of the mask is now TRUE.
* all interfaces should be allowed. */
dev->mask = ~dev->mask;
Not to mention that the accepted style for multi-line comments is
like this:
/*
* This is a
* long comment.
*/
Also, the kernel doesn't use CamelCase names like intfNr.
All these issues will have to be fixed before the patches can be
accepted.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
