reason (why):
for function uhci_sprint_schedule:
the buffer len is MAX_OUTPUT: 64 * 1024
the buffer 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
goal (what):
this patch fixes this correctness issue.
design (and why):
at first, we make enough room for buffering the exceeding contents
judge the contents which we have written whether bigger than buffer length
if bigger (the exceeding contents will be in the exceeding buffer)
break current work flow, and return
test:
plan:
let MAX_OUTPUT as various values:
some values which are enough for use, then can get full contents.
some values which small enough, so can truncate in various locations.
check the result:
cat the contents from the /sys/kernel/debug/usb/uhci/*
use wc -c to get the count of output contents (match the MAX_OUTPUT)
result:
make the buffer size in different size: (1024 is the room for exceeding)
63 * 1024 + 1024
1st (debug == 3) pass
2nd (debug == 3) pass
3rd (debug == 1) pass
4rd (not define DEBUG) pass
1689 + 1024 (debug == 3) pass (1689 is full contents length of my test)
1688 + 1024 (debug == 3) pass
1024 + 1024 (debug == 3) pass
512 + 1024 (debug == 3) pass
30 + 1024 (debug == 3) pass
left:
not test it by calling from uhci-q.c and uhci-hcd.c
not test the dump objects which already have rich data contents
if testing them are necessary:
please tell me, I will try.
and it is better to tell me how to test them.
Signed-off-by: Chen Gang <gang.chen@xxxxxxxxxxx>
---
drivers/usb/host/uhci-debug.c | 140 +++++++++++++++++++++++++++++------------
drivers/usb/host/uhci-hcd.c | 4 +-
drivers/usb/host/uhci-q.c | 2 +-
3 files changed, 102 insertions(+), 44 deletions(-)
diff --git a/drivers/usb/host/uhci-debug.c b/drivers/usb/host/uhci-debug.c
index fc0b0da..4951a1c 100644
--- a/drivers/usb/host/uhci-debug.c
+++ b/drivers/usb/host/uhci-debug.c
@@ -16,6 +16,8 @@
#include "uhci-hcd.h"
+#define LEFT_OUTPUT (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 truncate;
token = td_token(uhci, td);
switch (uhci_packetid(token)) {
@@ -90,7 +90,11 @@ 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));
+tail:
return out - buf;
+truncate:
+ out += sprintf(out, " (%s truncated)\n", __FUNCTION__);
+ goto tail;
}
static int uhci_show_urbp(struct uhci_hcd *uhci, struct urb_priv *urbp,
@@ -101,8 +105,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 +112,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 truncate;
switch (usb_pipetype(urbp->urb->pipe)) {
case PIPE_ISOCHRONOUS: ptype = "ISO"; break;
@@ -128,6 +132,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 truncate;
+
i = nactive = ninactive = 0;
list_for_each_entry(td, &urbp->td_list, list) {
if (urbp->qh->type != USB_ENDPOINT_XFER_ISOC &&
@@ -135,6 +142,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 truncate;
} else {
if (td_status(uhci, td) & TD_CTRL_ACTIVE)
++nactive;
@@ -146,8 +155,11 @@ 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);
-
+tail:
return out - buf;
+truncate:
+ out += sprintf(out, " (%s truncated)\n", __FUNCTION__);
+ goto tail;
}
static int uhci_show_qh(struct uhci_hcd *uhci,
@@ -158,10 +170,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 +190,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 truncate;
if (element & UHCI_PTR_QH(uhci))
out += sprintf(out, "%*s Element points to QH (bug?)\n", space, "");
@@ -195,6 +205,9 @@ 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 truncate;
+
if (list_empty(&qh->queue)) {
out += sprintf(out, "%*s queue is empty\n", space, "");
if (qh == uhci->skel_async_qh)
@@ -211,9 +224,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 truncate;
+ }
else
++nurbs;
}
@@ -222,24 +238,25 @@ static int uhci_show_qh(struct uhci_hcd *uhci,
space, "", nurbs);
}
+ if (out - buf > len)
+ goto truncate;
+
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);
}
+tail:
return out - buf;
+truncate:
+ out += sprintf(out, " (%s truncated)\n", __FUNCTION__);
+ goto tail;
}
-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 +269,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 +293,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 +305,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 +325,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 truncate;
out += sprintf(out, " usbstat = %04x %s%s%s%s%s%s\n",
usbstat,
@@ -328,20 +336,35 @@ 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 truncate;
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));
+ if (out - buf > len)
+ goto truncate;
+
+ out += uhci_show_sc(1, portsc1, out);
+ if (out - buf > len)
+ goto truncate;
+
+ out += uhci_show_sc(2, portsc2, out);
+ if (out - buf > len)
+ goto truncate;
+
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);
+tail:
return out - buf;
+truncate:
+ out += sprintf(out, " (%s truncated)\n", __FUNCTION__);
+ goto tail;
}
static int uhci_sprint_schedule(struct uhci_hcd *uhci, char *buf, int len)
@@ -360,9 +383,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 truncate;
out += sprintf(out, "HC status\n");
out += uhci_show_status(uhci, out, len - (out - buf));
+ if (out - buf > len)
+ goto truncate;
out += sprintf(out, "Periodic load table\n");
for (i = 0; i < MAX_PHASE; ++i) {
@@ -375,14 +402,19 @@ 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");
+ if (out - buf > len)
+ goto truncate;
+
nframes = 10;
nerrs = 0;
for (i = 0; i < UHCI_NUMFRAMES; ++i) {
__hc32 qh_dma;
+ if (out - buf > len)
+ goto truncate;
j = 0;
td = uhci->frame_cpu[i];
link = uhci->frame[i];
@@ -410,6 +442,9 @@ static int uhci_sprint_schedule(struct uhci_hcd *uhci, char *buf, int len)
if (nframes > 0)
out += uhci_show_td(uhci, td, out,
len - (out - buf), 4);
+ if (out - buf > len)
+ goto truncate;
+
link = td->link;
} while (tmp != head);
@@ -426,6 +461,8 @@ check_link:
out += sprintf(out, " link does not match "
"QH (%08x)!\n",
hc32_to_cpu(uhci, qh_dma));
+ if (out - buf > len)
+ goto truncate;
} else
++nerrs;
}
@@ -436,6 +473,9 @@ check_link:
out += sprintf(out, "Skeleton QHs\n");
+ if (out - buf > len)
+ goto truncate;
+
fsbr_link = 0;
for (i = 0; i < UHCI_NUM_SKELQH; ++i) {
int cnt = 0;
@@ -443,11 +483,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 truncate;
/* 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 truncate;
link = fsbr_link;
if (!link)
link = LINK_TO_QH(uhci, uhci->skel_term_qh);
@@ -460,9 +505,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 truncate;
+ }
if (!fsbr_link && qh->skel >= SKEL_FSBR)
fsbr_link = LINK_TO_QH(uhci, qh);
}
@@ -480,10 +528,19 @@ 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 truncate;
}
+tail:
return out - buf;
+
+truncate:
+ out += sprintf(out, " (main print truncated)\n");
+ goto tail;
}
#ifdef CONFIG_DEBUG_FS
@@ -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 - LEFT_OUTPUT);
spin_unlock_irqrestore(&uhci->lock, flags);
file->private_data = up;
diff --git a/drivers/usb/host/uhci-hcd.c b/drivers/usb/host/uhci-hcd.c
index 4b9e9ab..61e7a72 100644
--- a/drivers/usb/host/uhci-hcd.c
+++ b/drivers/usb/host/uhci-hcd.c
@@ -462,8 +462,8 @@ static irqreturn_t uhci_irq(struct usb_hcd *hcd)
"very bad!\n");
if (debug > 1 && errbuf) {
/* Print the schedule for debugging */
- uhci_sprint_schedule(uhci,
- errbuf, ERRBUF_LEN);
+ uhci_sprint_schedule(uhci, errbuf,
+ ERRBUF_LEN - LEFT_OUTPUT);
lprintk(errbuf);
}
uhci_hc_died(uhci);
diff --git a/drivers/usb/host/uhci-q.c b/drivers/usb/host/uhci-q.c
index 15921fd..83dda42 100644
--- a/drivers/usb/host/uhci-q.c
+++ b/drivers/usb/host/uhci-q.c
@@ -1200,7 +1200,7 @@ static int uhci_result_common(struct uhci_hcd *uhci, struct urb *urb)
if (debug > 1 && errbuf) {
/* Print the chain for debugging */
uhci_show_qh(uhci, urbp->qh, errbuf,
- ERRBUF_LEN, 0);
+ ERRBUF_LEN - LEFT_OUTPUT, 0);
lprintk(errbuf);
}
}
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
