From 13996ca7afd5b5d7980ea013b00e3ef7cf2cefd0 Mon Sep 17 00:00:00 2001 From: Chen Gang Date: Wed, 23 Jan 2013 16:13:41 +0800 Subject: USB: uhci: check buffer length to avoid memory overflow for function uhci_sprint_schedule: the buffer len is MAX_OUTPUT: 64 * 1024, which may not be enough: may loop UHCI_NUMFRAMES times (UHCI_NUMFRAMES is 1024) each time of loop may get more than 64 bytes so need check the buffer length to avoid memory overflow this patch fix it like this: at first, make enough room for buffering the exceeding contents judge the contents which written whether bigger than buffer length if bigger (the exceeding contents will be in the exceeding buffer) break current work flow, and return. Signed-off-by: Chen Gang Acked-by: Alan Stern Signed-off-by: Greg Kroah-Hartman --- drivers/usb/host/uhci-debug.c | 150 +++++++++++++++++++++++++++++------------- 1 file changed, 104 insertions(+), 46 deletions(-) (limited to 'drivers/usb/host/uhci-debug.c') diff --git a/drivers/usb/host/uhci-debug.c b/drivers/usb/host/uhci-debug.c index fc0b0daac93d..8a55bb25315b 100644 --- a/drivers/usb/host/uhci-debug.c +++ b/drivers/usb/host/uhci-debug.c @@ -16,6 +16,8 @@ #include "uhci-hcd.h" +#define EXTRA_SPACE 1024 + static struct dentry *uhci_debugfs_root; #ifdef DEBUG @@ -44,10 +46,6 @@ static int uhci_show_td(struct uhci_hcd *uhci, struct uhci_td *td, char *buf, char *spid; u32 status, token; - /* Try to make sure there's enough memory */ - if (len < 160) - return 0; - status = td_status(uhci, td); out += sprintf(out, "%*s[%p] link (%08x) ", space, "", td, hc32_to_cpu(uhci, td->link)); @@ -64,6 +62,8 @@ static int uhci_show_td(struct uhci_hcd *uhci, struct uhci_td *td, char *buf, (status & TD_CTRL_CRCTIMEO) ? "CRC/Timeo " : "", (status & TD_CTRL_BITSTUFF) ? "BitStuff " : "", status & 0x7ff); + if (out - buf > len) + goto done; token = td_token(uhci, td); switch (uhci_packetid(token)) { @@ -90,6 +90,9 @@ static int uhci_show_td(struct uhci_hcd *uhci, struct uhci_td *td, char *buf, spid); out += sprintf(out, "(buf=%08x)\n", hc32_to_cpu(uhci, td->buffer)); +done: + if (out - buf > len) + out += sprintf(out, " ...\n"); return out - buf; } @@ -101,8 +104,6 @@ static int uhci_show_urbp(struct uhci_hcd *uhci, struct urb_priv *urbp, int i, nactive, ninactive; char *ptype; - if (len < 200) - return 0; out += sprintf(out, "urb_priv [%p] ", urbp); out += sprintf(out, "urb [%p] ", urbp->urb); @@ -110,6 +111,8 @@ static int uhci_show_urbp(struct uhci_hcd *uhci, struct urb_priv *urbp, out += sprintf(out, "Dev=%d ", usb_pipedevice(urbp->urb->pipe)); out += sprintf(out, "EP=%x(%s) ", usb_pipeendpoint(urbp->urb->pipe), (usb_pipein(urbp->urb->pipe) ? "IN" : "OUT")); + if (out - buf > len) + goto done; switch (usb_pipetype(urbp->urb->pipe)) { case PIPE_ISOCHRONOUS: ptype = "ISO"; break; @@ -128,6 +131,9 @@ static int uhci_show_urbp(struct uhci_hcd *uhci, struct urb_priv *urbp, out += sprintf(out, " Unlinked=%d", urbp->urb->unlinked); out += sprintf(out, "\n"); + if (out - buf > len) + goto done; + i = nactive = ninactive = 0; list_for_each_entry(td, &urbp->td_list, list) { if (urbp->qh->type != USB_ENDPOINT_XFER_ISOC && @@ -135,6 +141,8 @@ static int uhci_show_urbp(struct uhci_hcd *uhci, struct urb_priv *urbp, out += sprintf(out, "%*s%d: ", space + 2, "", i); out += uhci_show_td(uhci, td, out, len - (out - buf), 0); + if (out - buf > len) + goto tail; } else { if (td_status(uhci, td) & TD_CTRL_ACTIVE) ++nactive; @@ -146,7 +154,10 @@ static int uhci_show_urbp(struct uhci_hcd *uhci, struct urb_priv *urbp, out += sprintf(out, "%*s[skipped %d inactive and %d active " "TDs]\n", space, "", ninactive, nactive); - +done: + if (out - buf > len) + out += sprintf(out, " ...\n"); +tail: return out - buf; } @@ -158,10 +169,6 @@ static int uhci_show_qh(struct uhci_hcd *uhci, __hc32 element = qh_element(qh); char *qtype; - /* Try to make sure there's enough memory */ - if (len < 80 * 7) - return 0; - switch (qh->type) { case USB_ENDPOINT_XFER_ISOC: qtype = "ISO"; break; case USB_ENDPOINT_XFER_INT: qtype = "INT"; break; @@ -182,6 +189,8 @@ static int uhci_show_qh(struct uhci_hcd *uhci, else if (qh->type == USB_ENDPOINT_XFER_INT) out += sprintf(out, "%*s period %d phase %d load %d us\n", space, "", qh->period, qh->phase, qh->load); + if (out - buf > len) + goto done; if (element & UHCI_PTR_QH(uhci)) out += sprintf(out, "%*s Element points to QH (bug?)\n", space, ""); @@ -195,11 +204,17 @@ static int uhci_show_qh(struct uhci_hcd *uhci, if (!(element & ~(UHCI_PTR_QH(uhci) | UHCI_PTR_DEPTH(uhci)))) out += sprintf(out, "%*s Element is NULL (bug?)\n", space, ""); + if (out - buf > len) + goto done; + if (list_empty(&qh->queue)) { out += sprintf(out, "%*s queue is empty\n", space, ""); - if (qh == uhci->skel_async_qh) + if (qh == uhci->skel_async_qh) { out += uhci_show_td(uhci, uhci->term_td, out, len - (out - buf), 0); + if (out - buf > len) + goto tail; + } } else { struct urb_priv *urbp = list_entry(qh->queue.next, struct urb_priv, node); @@ -211,9 +226,12 @@ static int uhci_show_qh(struct uhci_hcd *uhci, space, ""); i = nurbs = 0; list_for_each_entry(urbp, &qh->queue, node) { - if (++i <= 10) + if (++i <= 10) { out += uhci_show_urbp(uhci, urbp, out, len - (out - buf), space + 2); + if (out - buf > len) + goto tail; + } else ++nurbs; } @@ -222,24 +240,27 @@ static int uhci_show_qh(struct uhci_hcd *uhci, space, "", nurbs); } + if (out - buf > len) + goto done; + if (qh->dummy_td) { out += sprintf(out, "%*s Dummy TD\n", space, ""); out += uhci_show_td(uhci, qh->dummy_td, out, len - (out - buf), 0); + if (out - buf > len) + goto tail; } +done: + if (out - buf > len) + out += sprintf(out, " ...\n"); +tail: return out - buf; } -static int uhci_show_sc(int port, unsigned short status, char *buf, int len) +static int uhci_show_sc(int port, unsigned short status, char *buf) { - char *out = buf; - - /* Try to make sure there's enough memory */ - if (len < 160) - return 0; - - out += sprintf(out, " stat%d = %04x %s%s%s%s%s%s%s%s%s%s\n", + return sprintf(buf, " stat%d = %04x %s%s%s%s%s%s%s%s%s%s\n", port, status, (status & USBPORTSC_SUSP) ? " Suspend" : "", @@ -252,19 +273,12 @@ static int uhci_show_sc(int port, unsigned short status, char *buf, int len) (status & USBPORTSC_PE) ? " Enabled" : "", (status & USBPORTSC_CSC) ? " ConnectChange" : "", (status & USBPORTSC_CCS) ? " Connected" : ""); - - return out - buf; } -static int uhci_show_root_hub_state(struct uhci_hcd *uhci, char *buf, int len) +static int uhci_show_root_hub_state(struct uhci_hcd *uhci, char *buf) { - char *out = buf; char *rh_state; - /* Try to make sure there's enough memory */ - if (len < 60) - return 0; - switch (uhci->rh_state) { case UHCI_RH_RESET: rh_state = "reset"; break; @@ -283,9 +297,8 @@ static int uhci_show_root_hub_state(struct uhci_hcd *uhci, char *buf, int len) default: rh_state = "?"; break; } - out += sprintf(out, "Root-hub state: %s FSBR: %d\n", + return sprintf(buf, "Root-hub state: %s FSBR: %d\n", rh_state, uhci->fsbr_is_on); - return out - buf; } static int uhci_show_status(struct uhci_hcd *uhci, char *buf, int len) @@ -296,9 +309,6 @@ static int uhci_show_status(struct uhci_hcd *uhci, char *buf, int len) unsigned char sof; unsigned short portsc1, portsc2; - /* Try to make sure there's enough memory */ - if (len < 80 * 9) - return 0; usbcmd = uhci_readw(uhci, 0); usbstat = uhci_readw(uhci, 2); @@ -319,6 +329,8 @@ static int uhci_show_status(struct uhci_hcd *uhci, char *buf, int len) (usbcmd & USBCMD_GRESET) ? "GRESET " : "", (usbcmd & USBCMD_HCRESET) ? "HCRESET " : "", (usbcmd & USBCMD_RS) ? "RS " : ""); + if (out - buf > len) + goto done; out += sprintf(out, " usbstat = %04x %s%s%s%s%s%s\n", usbstat, @@ -328,19 +340,33 @@ static int uhci_show_status(struct uhci_hcd *uhci, char *buf, int len) (usbstat & USBSTS_RD) ? "ResumeDetect " : "", (usbstat & USBSTS_ERROR) ? "USBError " : "", (usbstat & USBSTS_USBINT) ? "USBINT " : ""); + if (out - buf > len) + goto done; out += sprintf(out, " usbint = %04x\n", usbint); out += sprintf(out, " usbfrnum = (%d)%03x\n", (usbfrnum >> 10) & 1, 0xfff & (4*(unsigned int)usbfrnum)); out += sprintf(out, " flbaseadd = %08x\n", flbaseadd); out += sprintf(out, " sof = %02x\n", sof); - out += uhci_show_sc(1, portsc1, out, len - (out - buf)); - out += uhci_show_sc(2, portsc2, out, len - (out - buf)); - out += sprintf(out, "Most recent frame: %x (%d) " - "Last ISO frame: %x (%d)\n", + if (out - buf > len) + goto done; + + out += uhci_show_sc(1, portsc1, out); + if (out - buf > len) + goto done; + + out += uhci_show_sc(2, portsc2, out); + if (out - buf > len) + goto done; + + out += sprintf(out, + "Most recent frame: %x (%d) Last ISO frame: %x (%d)\n", uhci->frame_number, uhci->frame_number & 1023, uhci->last_iso_frame, uhci->last_iso_frame & 1023); +done: + if (out - buf > len) + out += sprintf(out, " ...\n"); return out - buf; } @@ -360,9 +386,13 @@ static int uhci_sprint_schedule(struct uhci_hcd *uhci, char *buf, int len) "int8", "int4", "int2", "async", "term" }; - out += uhci_show_root_hub_state(uhci, out, len - (out - buf)); + out += uhci_show_root_hub_state(uhci, out); + if (out - buf > len) + goto done; out += sprintf(out, "HC status\n"); out += uhci_show_status(uhci, out, len - (out - buf)); + if (out - buf > len) + goto tail; out += sprintf(out, "Periodic load table\n"); for (i = 0; i < MAX_PHASE; ++i) { @@ -375,7 +405,7 @@ static int uhci_sprint_schedule(struct uhci_hcd *uhci, char *buf, int len) uhci_to_hcd(uhci)->self.bandwidth_int_reqs, uhci_to_hcd(uhci)->self.bandwidth_isoc_reqs); if (debug <= 1) - return out - buf; + goto tail; out += sprintf(out, "Frame List\n"); nframes = 10; @@ -383,6 +413,8 @@ static int uhci_sprint_schedule(struct uhci_hcd *uhci, char *buf, int len) for (i = 0; i < UHCI_NUMFRAMES; ++i) { __hc32 qh_dma; + if (out - buf > len) + goto done; j = 0; td = uhci->frame_cpu[i]; link = uhci->frame[i]; @@ -401,15 +433,20 @@ static int uhci_sprint_schedule(struct uhci_hcd *uhci, char *buf, int len) td = list_entry(tmp, struct uhci_td, fl_list); tmp = tmp->next; if (link != LINK_TO_TD(uhci, td)) { - if (nframes > 0) + if (nframes > 0) { out += sprintf(out, " link does " "not match list entry!\n"); - else + if (out - buf > len) + goto done; + } else ++nerrs; } - if (nframes > 0) + if (nframes > 0) { out += uhci_show_td(uhci, td, out, len - (out - buf), 4); + if (out - buf > len) + goto tail; + } link = td->link; } while (tmp != head); @@ -426,6 +463,8 @@ check_link: out += sprintf(out, " link does not match " "QH (%08x)!\n", hc32_to_cpu(uhci, qh_dma)); + if (out - buf > len) + goto done; } else ++nerrs; } @@ -436,6 +475,9 @@ check_link: out += sprintf(out, "Skeleton QHs\n"); + if (out - buf > len) + goto done; + fsbr_link = 0; for (i = 0; i < UHCI_NUM_SKELQH; ++i) { int cnt = 0; @@ -443,11 +485,16 @@ check_link: qh = uhci->skelqh[i]; out += sprintf(out, "- skel_%s_qh\n", qh_names[i]); \ out += uhci_show_qh(uhci, qh, out, len - (out - buf), 4); + if (out - buf > len) + goto tail; /* Last QH is the Terminating QH, it's different */ if (i == SKEL_TERM) { - if (qh_element(qh) != LINK_TO_TD(uhci, uhci->term_td)) + if (qh_element(qh) != LINK_TO_TD(uhci, uhci->term_td)) { out += sprintf(out, " skel_term_qh element is not set to term_td!\n"); + if (out - buf > len) + goto done; + } link = fsbr_link; if (!link) link = LINK_TO_QH(uhci, uhci->skel_term_qh); @@ -460,9 +507,12 @@ check_link: while (tmp != head) { qh = list_entry(tmp, struct uhci_qh, node); tmp = tmp->next; - if (++cnt <= 10) + if (++cnt <= 10) { out += uhci_show_qh(uhci, qh, out, len - (out - buf), 4); + if (out - buf > len) + goto tail; + } if (!fsbr_link && qh->skel >= SKEL_FSBR) fsbr_link = LINK_TO_QH(uhci, qh); } @@ -481,8 +531,15 @@ check_link: check_qh_link: if (qh->link != link) out += sprintf(out, " last QH not linked to next skeleton!\n"); + + if (out - buf > len) + goto done; } +done: + if (out - buf > len) + out += sprintf(out, " ...\n"); +tail: return out - buf; } @@ -514,7 +571,8 @@ static int uhci_debug_open(struct inode *inode, struct file *file) up->size = 0; spin_lock_irqsave(&uhci->lock, flags); if (uhci->is_initialized) - up->size = uhci_sprint_schedule(uhci, up->data, MAX_OUTPUT); + up->size = uhci_sprint_schedule(uhci, up->data, + MAX_OUTPUT - EXTRA_SPACE); spin_unlock_irqrestore(&uhci->lock, flags); file->private_data = up; -- cgit From 3171fcabb16993d6501fab7723371f0f3d0c6840 Mon Sep 17 00:00:00 2001 From: Chen Gang Date: Thu, 24 Jan 2013 09:41:45 +0800 Subject: USB: uhci: beautify source code get rid of the line breaks in string constants. let comments within 80 with limitation. delete ' \' at the end of a statement. Signed-off-by: Chen Gang Acked-by: Alan Stern Signed-off-by: Greg Kroah-Hartman --- drivers/usb/host/uhci-debug.c | 28 ++++++++++++++++------------ drivers/usb/host/uhci-hcd.c | 27 +++++++++++++-------------- drivers/usb/host/uhci-hub.c | 4 ++-- 3 files changed, 31 insertions(+), 28 deletions(-) (limited to 'drivers/usb/host/uhci-debug.c') diff --git a/drivers/usb/host/uhci-debug.c b/drivers/usb/host/uhci-debug.c index 8a55bb25315b..455737546525 100644 --- a/drivers/usb/host/uhci-debug.c +++ b/drivers/usb/host/uhci-debug.c @@ -151,8 +151,8 @@ static int uhci_show_urbp(struct uhci_hcd *uhci, struct urb_priv *urbp, } } if (nactive + ninactive > 0) - out += sprintf(out, "%*s[skipped %d inactive and %d active " - "TDs]\n", + out += sprintf(out, + "%*s[skipped %d inactive and %d active TDs]\n", space, "", ninactive, nactive); done: if (out - buf > len) @@ -182,8 +182,8 @@ static int uhci_show_qh(struct uhci_hcd *uhci, hc32_to_cpu(uhci, qh->link), hc32_to_cpu(uhci, element)); if (qh->type == USB_ENDPOINT_XFER_ISOC) - out += sprintf(out, "%*s period %d phase %d load %d us, " - "frame %x desc [%p]\n", + out += sprintf(out, + "%*s period %d phase %d load %d us, frame %x desc [%p]\n", space, "", qh->period, qh->phase, qh->load, qh->iso_frame, qh->iso_packet_desc); else if (qh->type == USB_ENDPOINT_XFER_INT) @@ -434,8 +434,8 @@ static int uhci_sprint_schedule(struct uhci_hcd *uhci, char *buf, int len) tmp = tmp->next; if (link != LINK_TO_TD(uhci, td)) { if (nframes > 0) { - out += sprintf(out, " link does " - "not match list entry!\n"); + out += sprintf(out, + " link does not match list entry!\n"); if (out - buf > len) goto done; } else @@ -460,8 +460,8 @@ check_link: i, hc32_to_cpu(uhci, link)); j = 1; } - out += sprintf(out, " link does not match " - "QH (%08x)!\n", + out += sprintf(out, + " link does not match QH (%08x)!\n", hc32_to_cpu(uhci, qh_dma)); if (out - buf > len) goto done; @@ -483,7 +483,7 @@ check_link: int cnt = 0; qh = uhci->skelqh[i]; - out += sprintf(out, "- skel_%s_qh\n", qh_names[i]); \ + out += sprintf(out, "- skel_%s_qh\n", qh_names[i]); out += uhci_show_qh(uhci, qh, out, len - (out - buf), 4); if (out - buf > len) goto tail; @@ -491,7 +491,8 @@ check_link: /* Last QH is the Terminating QH, it's different */ if (i == SKEL_TERM) { if (qh_element(qh) != LINK_TO_TD(uhci, uhci->term_td)) { - out += sprintf(out, " skel_term_qh element is not set to term_td!\n"); + out += sprintf(out, + " skel_term_qh element is not set to term_td!\n"); if (out - buf > len) goto done; } @@ -530,7 +531,8 @@ check_link: link = LINK_TO_QH(uhci, uhci->skel_term_qh); check_qh_link: if (qh->link != link) - out += sprintf(out, " last QH not linked to next skeleton!\n"); + out += sprintf(out, + " last QH not linked to next skeleton!\n"); if (out - buf > len) goto done; @@ -587,7 +589,9 @@ static loff_t uhci_debug_lseek(struct file *file, loff_t off, int whence) up = file->private_data; - /* XXX: atomic 64bit seek access, but that needs to be fixed in the VFS */ + /* + * XXX: atomic 64bit seek access, but that needs to be fixed in the VFS + */ switch (whence) { case 0: new = off; diff --git a/drivers/usb/host/uhci-hcd.c b/drivers/usb/host/uhci-hcd.c index 7c12b260531b..01628e39cc8b 100644 --- a/drivers/usb/host/uhci-hcd.c +++ b/drivers/usb/host/uhci-hcd.c @@ -449,17 +449,16 @@ static irqreturn_t uhci_irq(struct usb_hcd *hcd) if (status & ~(USBSTS_USBINT | USBSTS_ERROR | USBSTS_RD)) { if (status & USBSTS_HSE) - dev_err(uhci_dev(uhci), "host system error, " - "PCI problems?\n"); + dev_err(uhci_dev(uhci), + "host system error, PCI problems?\n"); if (status & USBSTS_HCPE) - dev_err(uhci_dev(uhci), "host controller process " - "error, something bad happened!\n"); + dev_err(uhci_dev(uhci), + "host controller process error, something bad happened!\n"); if (status & USBSTS_HCH) { spin_lock(&uhci->lock); if (uhci->rh_state >= UHCI_RH_RUNNING) { dev_err(uhci_dev(uhci), - "host controller halted, " - "very bad!\n"); + "host controller halted, very bad!\n"); if (debug > 1 && errbuf) { /* Print the schedule for debugging */ uhci_sprint_schedule(uhci, errbuf, @@ -589,8 +588,8 @@ static int uhci_start(struct usb_hcd *hcd) UHCI_NUMFRAMES * sizeof(*uhci->frame), &uhci->frame_dma_handle, 0); if (!uhci->frame) { - dev_err(uhci_dev(uhci), "unable to allocate " - "consistent memory for frame list\n"); + dev_err(uhci_dev(uhci), + "unable to allocate consistent memory for frame list\n"); goto err_alloc_frame; } memset(uhci->frame, 0, UHCI_NUMFRAMES * sizeof(*uhci->frame)); @@ -598,8 +597,8 @@ static int uhci_start(struct usb_hcd *hcd) uhci->frame_cpu = kcalloc(UHCI_NUMFRAMES, sizeof(*uhci->frame_cpu), GFP_KERNEL); if (!uhci->frame_cpu) { - dev_err(uhci_dev(uhci), "unable to allocate " - "memory for frame pointers\n"); + dev_err(uhci_dev(uhci), + "unable to allocate memory for frame pointers\n"); goto err_alloc_frame_cpu; } @@ -734,8 +733,8 @@ static int uhci_rh_suspend(struct usb_hcd *hcd) */ else if (hcd->self.root_hub->do_remote_wakeup && uhci->resuming_ports) { - dev_dbg(uhci_dev(uhci), "suspend failed because a port " - "is resuming\n"); + dev_dbg(uhci_dev(uhci), + "suspend failed because a port is resuming\n"); rc = -EBUSY; } else suspend_rh(uhci, UHCI_RH_SUSPENDED); @@ -826,8 +825,8 @@ static int uhci_count_ports(struct usb_hcd *hcd) /* Anything greater than 7 is weird so we'll ignore it. */ if (port > UHCI_RH_MAXCHILD) { - dev_info(uhci_dev(uhci), "port count misdetected? " - "forcing to 2 ports\n"); + dev_info(uhci_dev(uhci), + "port count misdetected? forcing to 2 ports\n"); port = 2; } diff --git a/drivers/usb/host/uhci-hub.c b/drivers/usb/host/uhci-hub.c index 768d54295a20..533b864f2e67 100644 --- a/drivers/usb/host/uhci-hub.c +++ b/drivers/usb/host/uhci-hub.c @@ -21,8 +21,8 @@ static const __u8 root_hub_hub_des[] = 0x00, /* (per-port OC, no power switching) */ 0x01, /* __u8 bPwrOn2pwrGood; 2ms */ 0x00, /* __u8 bHubContrCurrent; 0 mA */ - 0x00, /* __u8 DeviceRemovable; *** 7 Ports max *** */ - 0xff /* __u8 PortPwrCtrlMask; *** 7 ports max *** */ + 0x00, /* __u8 DeviceRemovable; *** 7 Ports max */ + 0xff /* __u8 PortPwrCtrlMask; *** 7 ports max */ }; #define UHCI_RH_MAXCHILD 7 -- cgit