linux-kernel - Re: [PATCH v2 1/3 RESEND] USB: serial: cp210x: New 16-bit register access functions.

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

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <5697D8FB.7000709@collabora.co.uk>
Date:	Thu, 14 Jan 2016 17:20:59 +0000
From:	Martyn Welch <martyn.welch@...labora.co.uk>
To:	Konstantin Shkolnyy <Konstantin.Shkolnyy@...abs.com>,
	Konstantin Shkolnyy <konstantin.shkolnyy@...il.com>,
	"johan@...nel.org" <johan@...nel.org>
CC:	"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 1/3 RESEND] USB: serial: cp210x: New 16-bit register
 access functions.



On 14/01/16 17:10, Konstantin Shkolnyy wrote:
>> -----Original Message-----
>> From: linux-usb-owner@...r.kernel.org [mailto:linux-usb-
>> owner@...r.kernel.org] On Behalf Of Martyn Welch
>> Sent: Thursday, January 14, 2016 10:31
>> To: Konstantin Shkolnyy; johan@...nel.org
>> Cc: linux-usb@...r.kernel.org; linux-kernel@...r.kernel.org
>> Subject: Re: [PATCH v2 1/3 RESEND] USB: serial: cp210x: New 16-bit register
>> access functions.
>>
>> On 23/12/15 02:14, Konstantin Shkolnyy wrote:
>>> cp210x_get_config and cp210x_set_config are cumbersome to use. This
>> change
>>> introduces new register access functions for 16-bit values, instead of
>>> the above functions.
>>>
>>> Signed-off-by: Konstantin Shkolnyy <konstantin.shkolnyy@...il.com>
>>> ---
>>>    drivers/usb/serial/cp210x.c | 155
>> +++++++++++++++++++++++++++++++-------------
>>>    1 file changed, 111 insertions(+), 44 deletions(-)
>>>
>>> diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
>>> index fd67958..fd7c4f4 100644
>>> --- a/drivers/usb/serial/cp210x.c
>>> +++ b/drivers/usb/serial/cp210x.c
>>> @@ -422,14 +422,88 @@ static int cp210x_set_config(struct
>> usb_serial_port *port, u8 request,
>>>    }
>>>
>>>    /*
>>> - * cp210x_set_config_single
>>> - * Convenience function for calling cp210x_set_config on single data
>> values
>>> - * without requiring an integer pointer
>>> + * Reads a variable-sized block of CP210X_ registers, identified by req.
>>> + * Returns data into buf in native USB byte order.
>>>     */
>>> -static inline int cp210x_set_config_single(struct usb_serial_port *port,
>>> -		u8 request, unsigned int data)
>>> +static int cp210x_read_reg_block(struct usb_serial_port *port, u8 req,
>>> +		void *buf, int bufsize)
>>>    {
>>> -	return cp210x_set_config(port, request, &data, 2);
>>> +	struct usb_serial *serial = port->serial;
>>> +	struct cp210x_port_private *port_priv =
>> usb_get_serial_port_data(port);
>>> +	void *dmabuf;
>>> +	int result;
>>> +
>>> +	dmabuf = kmalloc(bufsize, GFP_KERNEL);
>>> +	if (!dmabuf) {
>>> +		/*
>>> +		 * FIXME Some callers don't bother to check for error,
>>> +		 * at least give them consistent junk until they are fixed
>>> +		 */
>>> +		memset(buf, 0, bufsize);
>>> +		return -ENOMEM;
>>> +	}
>>> +
>>> +	result = usb_control_msg(serial->dev, usb_rcvctrlpipe(serial->dev,
>> 0),
>>> +			req, REQTYPE_INTERFACE_TO_HOST, 0,
>>> +			port_priv->bInterfaceNumber, dmabuf, bufsize,
>>> +			USB_CTRL_SET_TIMEOUT);
>>> +	if (result == bufsize) {
>>> +		memcpy(buf, dmabuf, bufsize);
>>> +		result = 0;
>>
>> I could be wrong, but isn't dmabuf a little-endian stream (hence the
>> byte order swapping in the existing set and get functions). Won't this
>> break on big endian systems due to the lack of byte swapping?
>
> This function is intended to not convert, as I tried to say in the header comment.
>

Ah, gotcha - conversion done in cp210x_read_u16_reg().

Martyn