Make sure that the interrupt-out endpoint max packet size is at least
eight bytes to avoid user-controlled slab corruption or NULL-pointer
dereference should a malicious device report a smaller size.

Fixes: 3416eaa1f8f8 ("USB: cypress_m8: Packet format is separate from characteristic size")
Cc: stable@vger.kernel.org	# 2.6.26
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/cypress_m8.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/usb/serial/cypress_m8.c b/drivers/usb/serial/cypress_m8.c
index afff1a0f4298..82ba0900b399 100644
--- a/drivers/usb/serial/cypress_m8.c
+++ b/drivers/usb/serial/cypress_m8.c
@@ -445,6 +445,14 @@ static int cypress_generic_port_probe(struct usb_serial_port *port)
 		return -ENODEV;
 	}
 
+	/*
+	 * The buffer must be large enough for the one or two-byte header (and
+	 * following data) but assume anything smaller than eight bytes is
+	 * broken.
+	 */
+	if (port->interrupt_out_size < 8)
+		return -EINVAL;
+
 	priv = kzalloc_obj(struct cypress_private);
 	if (!priv)
 		return -ENOMEM;
-- 
2.53.0

Hi Johan,

I took a closer look at your patch and tested it on top of commit
917719c412c4 with KASAN enabled.  I applied your patch, rebuilt the
kernel, and reran the same reproducer I used for the report.

The original reproducer still triggers:

  BUG: KASAN: slab-out-of-bounds in cypress_read_int_callback+0x240/0x7f0
  Read of size 1

I reproduced this with the in-kernel dummy_hcd host and the raw_gadget
interface. The emulated Earthmate-compatible device uses:

idVendor = 0x1163
idProduct = 0x0200
interrupt-in ep = 0x81, wMaxPacketSize = 1
interrupt-out ep = 0x02, wMaxPacketSize = 16

To reproduce, boot a KASAN kernel with dummy_hcd, raw_gadget,
usbserial, and cypress_m8 enabled, then run:

  modprobe -r g_zero g_serial g_mass_storage 2>/dev/null || true
  modprobe dummy_hcd raw_gadget usbmon usbserial cypress_m8 || true
  gcc -Wall -Wextra -O2 -o cypress_minigadget cypress_minigadget.c
  ./cypress_minigadget &
  for i in $(seq 1 160); do
          [ -e /dev/ttyUSB0 ] && break
          sleep 0.25
  done
  sh -c 'exec 3<>/dev/ttyUSB0; sleep 6; exec 3>&-'

The full reproducer is included below.

I think the reason is that your patch rejects small interrupt-out
endpoint sizes, but this reproducer has interrupt_out_size = 16, so the
new check is not hit.  The remaining issue is on the read side:
packet_format_1 reads data[1] before checking that urb->actual_length
contains the two-byte header.

I also tested a variant with interrupt-out wMaxPacketSize = 1.  Your
patch rejects that device during port probe with -EINVAL before ttyUSB0
is exposed, so the new check works for that endpoint-size case.

Please let me know if I missed anything in the test setup or in the
analysis above.  I am happy to help test another version, or send a
follow-up patch for cypress_read_int_callback() using your earlier
comments if that would be useful.

Best regards,
Zhang Cen

---- reproducer: cypress_minigadget.c ----
#define _GNU_SOURCE

#include <endian.h>
#include <errno.h>
#include <fcntl.h>
#include <signal.h>
#include <stdbool.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/ioctl.h>
#include <unistd.h>

#include <linux/types.h>
#include <linux/usb/ch9.h>

/*
 * Minimal Raw Gadget UAPI copied into this standalone harness so the guest
 * can compile it without depending on a matching linux-libc-dev package.
 */
#define UDC_NAME_LENGTH_MAX 128
#define USB_RAW_EPS_NUM_MAX 30
#define USB_RAW_EP_NAME_MAX 16
#define USB_RAW_EP_ADDR_ANY 0xff

struct usb_raw_init {
__u8 driver_name[UDC_NAME_LENGTH_MAX];
__u8 device_name[UDC_NAME_LENGTH_MAX];
__u8 speed;
};

enum usb_raw_event_type {
USB_RAW_EVENT_INVALID = 0,
USB_RAW_EVENT_CONNECT = 1,
USB_RAW_EVENT_CONTROL = 2,
USB_RAW_EVENT_SUSPEND = 3,
USB_RAW_EVENT_RESUME = 4,
USB_RAW_EVENT_RESET = 5,
USB_RAW_EVENT_DISCONNECT = 6,
};

struct usb_raw_event {
__u32 type;
__u32 length;
__u8 data[];
};

struct usb_raw_ep_io {
__u16 ep;
__u16 flags;
__u32 length;
__u8 data[];
};

struct usb_raw_ep_caps {
__u32 type_control : 1;
__u32 type_iso : 1;
__u32 type_bulk : 1;
__u32 type_int : 1;
__u32 dir_in : 1;
__u32 dir_out : 1;
};

struct usb_raw_ep_limits {
__u16 maxpacket_limit;
__u16 max_streams;
__u32 reserved;
};

struct usb_raw_ep_info {
__u8 name[USB_RAW_EP_NAME_MAX];
__u32 addr;
struct usb_raw_ep_caps caps;
struct usb_raw_ep_limits limits;
};

struct usb_raw_eps_info {
struct usb_raw_ep_info eps[USB_RAW_EPS_NUM_MAX];
};

#define USB_RAW_IOCTL_INIT _IOW('U', 0, struct usb_raw_init)
#define USB_RAW_IOCTL_RUN _IO('U', 1)
#define USB_RAW_IOCTL_EVENT_FETCH _IOR('U', 2, struct usb_raw_event)
#define USB_RAW_IOCTL_EP0_WRITE _IOW('U', 3, struct usb_raw_ep_io)
#define USB_RAW_IOCTL_EP0_READ _IOWR('U', 4, struct usb_raw_ep_io)
#define USB_RAW_IOCTL_EP_ENABLE _IOW('U', 5, struct usb_endpoint_descriptor)
#define USB_RAW_IOCTL_EP_WRITE _IOW('U', 7, struct usb_raw_ep_io)
#define USB_RAW_IOCTL_CONFIGURE _IO('U', 9)
#define USB_RAW_IOCTL_EPS_INFO _IOR('U', 11, struct usb_raw_eps_info)
#define USB_RAW_IOCTL_EP0_STALL _IO('U', 12)

#define VENDOR_ID 0x1163
#define PRODUCT_ID 0x0200
#define CONFIG_VALUE 1
#define IN_MAX_PACKET 1
#define OUT_MAX_PACKET 16
#define SET_REPORT 0x09
#define GET_REPORT 0x01

struct descriptor_block {
struct usb_config_descriptor config;
struct usb_interface_descriptor intf;
struct usb_endpoint_descriptor ep_in;
struct usb_endpoint_descriptor ep_out;
} __attribute__((packed));

static int raw_fd = -1;
static int ep_in_handle = -1;
static int ep_out_handle = -1;
static uint8_t ep_in_addr = 0x81;
static uint8_t ep_out_addr = 0x02;
static bool endpoints_enabled;
static bool configuration_done;
static bool packet_sent;
static int connect_count;
static int disconnect_count;
static volatile sig_atomic_t stop_requested;

static void on_signal(int signo)
{
(void)signo;
stop_requested = 1;
}

static void print_hex(const char *label, const uint8_t *buf, size_t len)
{
size_t i;

fprintf(stderr, "%s (%zu bytes):", label, len);
for (i = 0; i < len; ++i)
fprintf(stderr, " %02x", buf[i]);
fprintf(stderr, "\n");
}

static int do_ioctl(unsigned long request, void *arg, const char *what)
{
int ret = ioctl(raw_fd, request, arg);

if (ret < 0)
fprintf(stderr, "%s failed: %s\n", what, strerror(errno));
return ret;
}

static int ep0_io(unsigned long request, uint8_t *buf, size_t len)
{
struct usb_raw_ep_io *io;
size_t total = sizeof(*io) + len;
int ret;

io = calloc(1, total);
if (!io)
return -ENOMEM;
io->length = len;
if (request == USB_RAW_IOCTL_EP0_WRITE && len)
memcpy(io->data, buf, len);

ret = ioctl(raw_fd, request, io);
if (ret < 0) {
ret = -errno;
fprintf(stderr, "ep0 ioctl 0x%lx failed: %s\n",
request, strerror(errno));
goto out;
}
if (request == USB_RAW_IOCTL_EP0_READ && ret > 0 && buf)
memcpy(buf, io->data, ret);
out:
free(io);
return ret;
}

static int ep_io(unsigned long request, uint16_t handle, uint8_t *buf,
size_t len)
{
struct usb_raw_ep_io *io;
size_t total = sizeof(*io) + len;
int ret;

io = calloc(1, total);
if (!io)
return -ENOMEM;
io->ep = handle;
io->length = len;
if (request == USB_RAW_IOCTL_EP_WRITE && len)
memcpy(io->data, buf, len);

ret = ioctl(raw_fd, request, io);
if (ret < 0) {
ret = -errno;
fprintf(stderr, "ep ioctl 0x%lx handle %u failed: %s\n",
request, handle, strerror(errno));
goto out;
}
out:
free(io);
return ret;
}

static int ep0_write_bytes(const void *buf, size_t len)
{
return ep0_io(USB_RAW_IOCTL_EP0_WRITE, (uint8_t *)buf, len);
}

static int ep0_read_bytes(uint8_t *buf, size_t len)
{
return ep0_io(USB_RAW_IOCTL_EP0_READ, buf, len);
}

static int ep_write_bytes(uint16_t handle, const void *buf, size_t len)
{
return ep_io(USB_RAW_IOCTL_EP_WRITE, handle, (uint8_t *)buf, len);
}

static int ep0_stall(void)
{
if (ioctl(raw_fd, USB_RAW_IOCTL_EP0_STALL, 0) < 0) {
fprintf(stderr, "ep0 stall failed: %s\n", strerror(errno));
return -errno;
}
return 0;
}

static size_t build_string_descriptor(const char *ascii, uint8_t *out,
size_t out_len)
{
size_t ascii_len = strlen(ascii);
size_t needed = 2 + ascii_len * 2;
size_t i;

if (out_len < needed)
return 0;

out[0] = needed;
out[1] = USB_DT_STRING;
for (i = 0; i < ascii_len; ++i) {
out[2 + i * 2] = (uint8_t)ascii[i];
out[3 + i * 2] = 0;
}

return needed;
}

static void choose_endpoints(void)
{
struct usb_raw_eps_info info;
int count;
int i;
bool any_in = false;
bool any_out = false;

memset(&info, 0, sizeof(info));
count = do_ioctl(USB_RAW_IOCTL_EPS_INFO, &info, "USB_RAW_IOCTL_EPS_INFO");
if (count < 0)
exit(1);

fprintf(stderr, "raw gadget reports %d endpoints\n", count);
for (i = 0; i < count && i < USB_RAW_EPS_NUM_MAX; ++i) {
char name[USB_RAW_EP_NAME_MAX + 1];
struct usb_raw_ep_info *ep = &info.eps[i];

memcpy(name, ep->name, USB_RAW_EP_NAME_MAX);
name[USB_RAW_EP_NAME_MAX] = '\0';
fprintf(stderr,
"  ep[%d] name=%s addr=0x%02x int=%u in=%u out=%u maxpacket=%u\n",
i, name, ep->addr, ep->caps.type_int, ep->caps.dir_in,
ep->caps.dir_out, ep->limits.maxpacket_limit);

if (ep->caps.type_int && ep->caps.dir_in) {
if (ep->addr == USB_RAW_EP_ADDR_ANY)
any_in = true;
else if (!any_in && ep_in_addr == 0x81)
ep_in_addr = ep->addr;
}
if (ep->caps.type_int && ep->caps.dir_out) {
if (ep->addr == USB_RAW_EP_ADDR_ANY)
any_out = true;
else if (!any_out && ep_out_addr == 0x02)
ep_out_addr = ep->addr;
}
}

if (any_in)
ep_in_addr = 0x81;
if (any_out)
ep_out_addr = 0x02;

fprintf(stderr, "using interrupt endpoints in=0x%02x out=0x%02x\n",
ep_in_addr, ep_out_addr);
}

static struct descriptor_block make_config_block(void)
{
struct descriptor_block block;

memset(&block, 0, sizeof(block));

block.config.bLength = sizeof(block.config);
block.config.bDescriptorType = USB_DT_CONFIG;
block.config.wTotalLength = htole16(sizeof(block));
block.config.bNumInterfaces = 1;
block.config.bConfigurationValue = CONFIG_VALUE;
block.config.bmAttributes = USB_CONFIG_ATT_ONE;
block.config.bMaxPower = 1;

block.intf.bLength = sizeof(block.intf);
block.intf.bDescriptorType = USB_DT_INTERFACE;
block.intf.bInterfaceNumber = 0;
block.intf.bAlternateSetting = 0;
block.intf.bNumEndpoints = 2;
block.intf.bInterfaceClass = USB_CLASS_VENDOR_SPEC;
block.intf.bInterfaceSubClass = 0;
block.intf.bInterfaceProtocol = 0;

block.ep_in.bLength = sizeof(block.ep_in);
block.ep_in.bDescriptorType = USB_DT_ENDPOINT;
block.ep_in.bEndpointAddress = ep_in_addr;
block.ep_in.bmAttributes = USB_ENDPOINT_XFER_INT;
block.ep_in.wMaxPacketSize = htole16(IN_MAX_PACKET);
block.ep_in.bInterval = 1;

block.ep_out.bLength = sizeof(block.ep_out);
block.ep_out.bDescriptorType = USB_DT_ENDPOINT;
block.ep_out.bEndpointAddress = ep_out_addr;
block.ep_out.bmAttributes = USB_ENDPOINT_XFER_INT;
block.ep_out.wMaxPacketSize = htole16(OUT_MAX_PACKET);
block.ep_out.bInterval = 1;

return block;
}

static int ensure_endpoints_enabled(void)
{
struct descriptor_block block = make_config_block();

if (endpoints_enabled)
return 0;

ep_in_handle = ioctl(raw_fd, USB_RAW_IOCTL_EP_ENABLE, &block.ep_in);
if (ep_in_handle < 0) {
fprintf(stderr, "enable IN endpoint failed: %s\n", strerror(errno));
return -errno;
}

ep_out_handle = ioctl(raw_fd, USB_RAW_IOCTL_EP_ENABLE, &block.ep_out);
if (ep_out_handle < 0) {
fprintf(stderr, "enable OUT endpoint failed: %s\n", strerror(errno));
return -errno;
}

if (ioctl(raw_fd, USB_RAW_IOCTL_CONFIGURE, 0) < 0) {
fprintf(stderr, "USB_RAW_IOCTL_CONFIGURE failed: %s\n",
strerror(errno));
return -errno;
}

endpoints_enabled = true;
fprintf(stderr,
"configured endpoints: in_handle=%d out_handle=%d in_addr=0x%02x
out_addr=0x%02x\n",
ep_in_handle, ep_out_handle, ep_in_addr, ep_out_addr);

return 0;
}

static int handle_get_descriptor(uint16_t value, uint16_t length)
{
uint8_t desc_type = value >> 8;
uint8_t desc_index = value & 0xff;
uint8_t buffer[256];
size_t send_len = 0;
struct usb_device_descriptor device = {
.bLength = USB_DT_DEVICE_SIZE,
.bDescriptorType = USB_DT_DEVICE,
.bcdUSB = htole16(0x0200),
.bDeviceClass = USB_CLASS_PER_INTERFACE,
.bDeviceSubClass = 0,
.bDeviceProtocol = 0,
.bMaxPacketSize0 = 64,
.idVendor = htole16(VENDOR_ID),
.idProduct = htole16(PRODUCT_ID),
.bcdDevice = htole16(0x0001),
.iManufacturer = 1,
.iProduct = 2,
.iSerialNumber = 3,
.bNumConfigurations = 1,
};
struct descriptor_block config = make_config_block();

memset(buffer, 0, sizeof(buffer));

switch (desc_type) {
case USB_DT_DEVICE:
memcpy(buffer, &device, sizeof(device));
send_len = sizeof(device);
break;
case USB_DT_CONFIG:
memcpy(buffer, &config, sizeof(config));
send_len = sizeof(config);
break;
case USB_DT_STRING:
if (desc_index == 0) {
buffer[0] = 4;
buffer[1] = USB_DT_STRING;
buffer[2] = 0x09;
buffer[3] = 0x04;
send_len = 4;
} else if (desc_index == 1) {
send_len = build_string_descriptor("raw-gadget", buffer,
sizeof(buffer));
} else if (desc_index == 2) {
send_len = build_string_descriptor("Earthmate short-header test",
buffer, sizeof(buffer));
} else if (desc_index == 3) {
send_len = build_string_descriptor("test-001", buffer,
sizeof(buffer));
}
break;
default:
fprintf(stderr, "stalling unsupported descriptor type 0x%02x index %u\n",
desc_type, desc_index);
return ep0_stall();
}

if (send_len == 0)
return ep0_stall();

if (length < send_len)
send_len = length;

print_hex("ep0 descriptor reply", buffer, send_len);
return ep0_write_bytes(buffer, send_len);
}

static int handle_standard_request(const struct usb_ctrlrequest *ctrl)
{
uint16_t value = le16toh(ctrl->wValue);
uint16_t index = le16toh(ctrl->wIndex);
uint16_t length = le16toh(ctrl->wLength);
uint8_t status[2] = { 0, 0 };
uint8_t alt = 0;

switch (ctrl->bRequest) {
case USB_REQ_GET_DESCRIPTOR:
return handle_get_descriptor(value, length);
case USB_REQ_SET_ADDRESS:
fprintf(stderr, "ack SET_ADDRESS %u\n", value);
return ep0_read_bytes(NULL, 0);
case USB_REQ_SET_CONFIGURATION:
fprintf(stderr, "SET_CONFIGURATION %u\n", value);
if (value == CONFIG_VALUE) {
int ret;

ret = ep0_read_bytes(NULL, 0);
if (ret < 0)
return ret;
ret = ensure_endpoints_enabled();
if (ret < 0)
return ret;
configuration_done = true;
return 0;
}
return ep0_read_bytes(NULL, 0);
case USB_REQ_GET_CONFIGURATION: {
uint8_t current_config = value;

(void)current_config;
current_config = configuration_done ? CONFIG_VALUE : 0;
return ep0_write_bytes(&current_config, sizeof(current_config));
}
case USB_REQ_GET_STATUS:
return ep0_write_bytes(status, sizeof(status));
case USB_REQ_SET_INTERFACE:
fprintf(stderr, "SET_INTERFACE index=%u alt=%u\n", index, value);
return ep0_read_bytes(NULL, 0);
case USB_REQ_GET_INTERFACE:
return ep0_write_bytes(&alt, sizeof(alt));
case USB_REQ_CLEAR_FEATURE:
case USB_REQ_SET_FEATURE:
fprintf(stderr, "ack feature request req=0x%02x value=0x%04x index=0x%04x\n",
ctrl->bRequest, value, index);
return ep0_read_bytes(NULL, 0);
default:
fprintf(stderr, "stalling unsupported standard request 0x%02x\n",
ctrl->bRequest);
return ep0_stall();
}
}

static int send_short_interrupt_packet(void)
{
uint8_t payload[1] = { 0 };
int ret;

if (packet_sent)
return 0;
if (!configuration_done || ep_in_handle < 0)
return -EINVAL;

fprintf(stderr,
"queueing 1-byte interrupt-in transfer: handle=%d addr=0x%02x maxpacket=%u\n",
ep_in_handle, ep_in_addr, IN_MAX_PACKET);
ret = ep_write_bytes(ep_in_handle, payload, sizeof(payload));
if (ret < 0)
return ret;

packet_sent = true;
fprintf(stderr, "interrupt-in transfer completed with %d byte(s)\n", ret);
return ret;
}

static int handle_class_request(const struct usb_ctrlrequest *ctrl)
{
uint16_t length = le16toh(ctrl->wLength);
uint8_t *buffer;
int ret;

buffer = calloc(1, length ? length : 1);
if (!buffer)
return -ENOMEM;

switch (ctrl->bRequest) {
case SET_REPORT:
ret = ep0_read_bytes(buffer, length);
if (ret >= 0) {
print_hex("SET_REPORT payload", buffer, ret);
ret = send_short_interrupt_packet();
if (ret >= 0)
ret = 0;
}
free(buffer);
return ret;
case GET_REPORT:
memset(buffer, 0, length);
print_hex("GET_REPORT reply", buffer, length);
ret = ep0_write_bytes(buffer, length);
free(buffer);
return ret;
default:
fprintf(stderr, "stalling unsupported class request 0x%02x\n",
ctrl->bRequest);
free(buffer);
return ep0_stall();
}
}

static int handle_control_event(const struct usb_ctrlrequest *ctrl)
{
fprintf(stderr,
"control request type=0x%02x req=0x%02x value=0x%04x index=0x%04x len=%u\n",
ctrl->bRequestType, ctrl->bRequest, le16toh(ctrl->wValue),
le16toh(ctrl->wIndex), le16toh(ctrl->wLength));

switch (ctrl->bRequestType & USB_TYPE_MASK) {
case USB_TYPE_STANDARD:
return handle_standard_request(ctrl);
case USB_TYPE_CLASS:
return handle_class_request(ctrl);
default:
fprintf(stderr, "stalling request with unsupported type mask 0x%02x\n",
ctrl->bRequestType & USB_TYPE_MASK);
return ep0_stall();
}
}

int main(void)
{
struct usb_raw_init init;
struct {
struct usb_raw_event event;
uint8_t data[256];
} event_buf;

signal(SIGINT, on_signal);
signal(SIGTERM, on_signal);
signal(SIGALRM, on_signal);
alarm(75);

raw_fd = open("/dev/raw-gadget", O_RDWR);
if (raw_fd < 0) {
perror("open /dev/raw-gadget");
return 1;
}

memset(&init, 0, sizeof(init));
memcpy(init.driver_name, "dummy_udc", sizeof("dummy_udc"));
memcpy(init.device_name, "dummy_udc.0", sizeof("dummy_udc.0"));
init.speed = USB_SPEED_FULL;

if (do_ioctl(USB_RAW_IOCTL_INIT, &init, "USB_RAW_IOCTL_INIT") < 0)
return 1;
if (do_ioctl(USB_RAW_IOCTL_RUN, NULL, "USB_RAW_IOCTL_RUN") < 0)
return 1;

fprintf(stderr,
"raw gadget started on dummy_udc.0 for VID:PID %04x:%04x\n",
VENDOR_ID, PRODUCT_ID);

while (!stop_requested) {
struct usb_ctrlrequest ctrl;
int ret;

memset(&event_buf, 0, sizeof(event_buf));
event_buf.event.length = sizeof(event_buf.data);
ret = ioctl(raw_fd, USB_RAW_IOCTL_EVENT_FETCH, &event_buf);
if (ret < 0) {
if (errno == EINTR && stop_requested)
break;
perror("USB_RAW_IOCTL_EVENT_FETCH");
return 1;
}

switch (event_buf.event.type) {
case USB_RAW_EVENT_CONNECT:
connect_count++;
fprintf(stderr, "received CONNECT event #%d\n", connect_count);
choose_endpoints();
break;
case USB_RAW_EVENT_CONTROL:
if (event_buf.event.length != sizeof(ctrl)) {
fprintf(stderr,
"unexpected control payload size %u\n",
event_buf.event.length);
return 1;
}
memcpy(&ctrl, event_buf.data, sizeof(ctrl));
ret = handle_control_event(&ctrl);
if (ret < 0) {
fprintf(stderr, "control handling failed: %d\n", ret);
return 1;
}
if (packet_sent) {
fprintf(stderr, "packet sent; waiting briefly for follow-up events\n");
sleep(2);
return 0;
}
break;
case USB_RAW_EVENT_RESET:
fprintf(stderr, "received RESET event\n");
break;
case USB_RAW_EVENT_DISCONNECT:
disconnect_count++;
fprintf(stderr, "received DISCONNECT event #%d\n", disconnect_count);
if (packet_sent)
return 0;
if (disconnect_count >= 4)
return 1;
break;
case USB_RAW_EVENT_SUSPEND:
fprintf(stderr, "received SUSPEND event\n");
break;
case USB_RAW_EVENT_RESUME:
fprintf(stderr, "received RESUME event\n");
break;
default:
fprintf(stderr, "received event type %u\n", event_buf.event.type);
break;
}
}

fprintf(stderr, "stop requested before packet was sent\n");
return packet_sent ? 0 : 1;
}

---- end reproducer ----

Greg Kroah-Hartman <gregkh@linuxfoundation.org> 于2026年5月22日周五 19:35写道：
>
> On Fri, May 22, 2026 at 12:16:21PM +0200, Johan Hovold wrote:
> > Make sure that the interrupt-out endpoint max packet size is at least
> > eight bytes to avoid user-controlled slab corruption or NULL-pointer
> > dereference should a malicious device report a smaller size.
> >
> > Fixes: 3416eaa1f8f8 ("USB: cypress_m8: Packet format is separate from characteristic size")
> > Cc: stable@vger.kernel.org    # 2.6.26
> > Signed-off-by: Johan Hovold <johan@kernel.org>
> > ---
> >  drivers/usb/serial/cypress_m8.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/usb/serial/cypress_m8.c b/drivers/usb/serial/cypress_m8.c
> > index afff1a0f4298..82ba0900b399 100644
> > --- a/drivers/usb/serial/cypress_m8.c
> > +++ b/drivers/usb/serial/cypress_m8.c
> > @@ -445,6 +445,14 @@ static int cypress_generic_port_probe(struct usb_serial_port *port)
> >               return -ENODEV;
> >       }
> >
> > +     /*
> > +      * The buffer must be large enough for the one or two-byte header (and
> > +      * following data) but assume anything smaller than eight bytes is
> > +      * broken.
> > +      */
> > +     if (port->interrupt_out_size < 8)
> > +             return -EINVAL;
> > +
> >       priv = kzalloc_obj(struct cypress_private);
> >       if (!priv)
> >               return -ENOMEM;
> > --
> > 2.53.0
> >
> >
>
> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Johan Hovold <johan@kernel.org> 于2026年5月22日周五 18:16写道：
>
> Make sure that the interrupt-out endpoint max packet size is at least
> eight bytes to avoid user-controlled slab corruption or NULL-pointer
> dereference should a malicious device report a smaller size.
>
> Fixes: 3416eaa1f8f8 ("USB: cypress_m8: Packet format is separate from characteristic size")
> Cc: stable@vger.kernel.org      # 2.6.26
> Signed-off-by: Johan Hovold <johan@kernel.org>
> ---
>  drivers/usb/serial/cypress_m8.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/drivers/usb/serial/cypress_m8.c b/drivers/usb/serial/cypress_m8.c
> index afff1a0f4298..82ba0900b399 100644
> --- a/drivers/usb/serial/cypress_m8.c
> +++ b/drivers/usb/serial/cypress_m8.c
> @@ -445,6 +445,14 @@ static int cypress_generic_port_probe(struct usb_serial_port *port)
>                 return -ENODEV;
>         }
>
> +       /*
> +        * The buffer must be large enough for the one or two-byte header (and
> +        * following data) but assume anything smaller than eight bytes is
> +        * broken.
> +        */
> +       if (port->interrupt_out_size < 8)
> +               return -EINVAL;
> +
>         priv = kzalloc_obj(struct cypress_private);
>         if (!priv)
>                 return -ENOMEM;
> --
> 2.53.0
>

On Fri, May 22, 2026 at 10:16:07PM +0800, Cen Zhang wrote:

> I took a closer look at your patch and tested it on top of commit
> 917719c412c4 with KASAN enabled.  I applied your patch, rebuilt the
> kernel, and reran the same reproducer I used for the report.
> 
> The original reproducer still triggers:
> 
>   BUG: KASAN: slab-out-of-bounds in cypress_read_int_callback+0x240/0x7f0
>   Read of size 1

> I think the reason is that your patch rejects small interrupt-out
> endpoint sizes, but this reproducer has interrupt_out_size = 16, so the
> new check is not hit.  The remaining issue is on the read side:
> packet_format_1 reads data[1] before checking that urb->actual_length
> contains the two-byte header.
 
Sorry if it wasn't clear but my patch isn't meant to replace yours as it
fixes a separate issue (introduced by the same commit).

> I also tested a variant with interrupt-out wMaxPacketSize = 1.  Your
> patch rejects that device during port probe with -EINVAL before ttyUSB0
> is exposed, so the new check works for that endpoint-size case.

Thanks for testing it.

> Please let me know if I missed anything in the test setup or in the
> analysis above.  I am happy to help test another version, or send a
> follow-up patch for cypress_read_int_callback() using your earlier
> comments if that would be useful.

I'm hoping you can send me a v2 of your fix.

Johan

Hi Johan,

> Sorry if it wasn't clear but my patch isn't meant to replace yours as it
> fixes a separate issue (introduced by the same commit).

Sorry for the misunderstanding regarding the intent of your patch.

> I'm hoping you can send me a v2 of your fix.
>
> Johan

I’ll work on a v2 of my fix and send it as soon as possible.

Thanks a lot for your guidance.

Best regards,
Zhang Cen

On Fri, May 22, 2026 at 12:16:21PM +0200, Johan Hovold wrote:
> Make sure that the interrupt-out endpoint max packet size is at least
> eight bytes to avoid user-controlled slab corruption or NULL-pointer
> dereference should a malicious device report a smaller size.
> 
> Fixes: 3416eaa1f8f8 ("USB: cypress_m8: Packet format is separate from characteristic size")
> Cc: stable@vger.kernel.org	# 2.6.26
> Signed-off-by: Johan Hovold <johan@kernel.org>
> ---
>  drivers/usb/serial/cypress_m8.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/usb/serial/cypress_m8.c b/drivers/usb/serial/cypress_m8.c
> index afff1a0f4298..82ba0900b399 100644
> --- a/drivers/usb/serial/cypress_m8.c
> +++ b/drivers/usb/serial/cypress_m8.c
> @@ -445,6 +445,14 @@ static int cypress_generic_port_probe(struct usb_serial_port *port)
>  		return -ENODEV;
>  	}
>  
> +	/*
> +	 * The buffer must be large enough for the one or two-byte header (and
> +	 * following data) but assume anything smaller than eight bytes is
> +	 * broken.
> +	 */
> +	if (port->interrupt_out_size < 8)
> +		return -EINVAL;
> +
>  	priv = kzalloc_obj(struct cypress_private);
>  	if (!priv)
>  		return -ENOMEM;
> -- 
> 2.53.0
> 
> 

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>