linux-kernel - Re: [PATCH v3] USB: serial: pl2303: account for deficits of clones

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: <ZyuZL7CGLqe3J_ck@hovoldconsulting.com>
Date: Wed, 6 Nov 2024 17:28:31 +0100
From: Johan Hovold <johan@...nel.org>
To: Jan Kiszka <jan.kiszka@...mens.com>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	linux-usb <linux-usb@...r.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Michał Pecio <michal.pecio@...il.com>
Subject: Re: [PATCH v3] USB: serial: pl2303: account for deficits of clones

On Thu, Sep 12, 2024 at 10:37:39AM +0200, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@...mens.com>
> 
> There are apparently incomplete clones of the HXD type chip in use.
> Those return -EPIPE on GET_LINE_REQUEST and BREAK_REQUEST. Avoid
> flooding the kernel log with those errors. Detect them during startup
> and then use the line_settings cache instead of GET_LINE_REQUEST. Signal
> missing break support via -ENOTTY.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@...mens.com>
> ---
> 
> Changes in v3:
>  - use quirks field instead of new type (suggested by Michal)

>  #define PL2303_QUIRK_UART_STATE_IDX0		BIT(0)
>  #define PL2303_QUIRK_LEGACY			BIT(1)
>  #define PL2303_QUIRK_ENDPOINT_HACK		BIT(2)
> +#define PL2304_QUIRK_NO_BREAK_GETLINE		BIT(3)

I believe you meant to use a "PL2303" prefix here as well.

> +static bool pl2303_is_hxd_clone(struct usb_serial *serial)
> +{
> +	struct usb_device *udev = serial->dev;
> +	unsigned char *buf;
> +	int ret;
> +
> +	buf = kmalloc(7, GFP_KERNEL);
> +	if (!buf) {
> +		dev_err(&serial->interface->dev,
> +			"could not check for clone type, not enough memory\n");

And there's no need to log out-of-memory errors (e.g. which would
already have been logged by allocator).

> +		return false;
> +	}
> +
> +	ret = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
> +			      GET_LINE_REQUEST, GET_LINE_REQUEST_TYPE,
> +			      0, 0, buf, 7, 100);
> +
> +	kfree(buf);
> +
> +	return ret == -EPIPE;
> +}

I fixed up the above when applying. Looks nice and clean otherwise.

Thanks.

Johan