* [PATCH 1/2] usb: host: uhci-debug: use scnprintf() instead of sprintf()
2022-03-12 20:28 [PATCH 0/2] Switch to using scnprintf() in the UHCI driver's debugging code Sergey Shtylyov
@ 2022-03-12 20:28 ` Sergey Shtylyov
2022-03-12 22:33 ` David Laight
2022-03-12 20:28 ` [PATCH 2/2] usb: host: uhci: remove #define EXTRA_SPACE Sergey Shtylyov
1 sibling, 1 reply; 5+ messages in thread
From: Sergey Shtylyov @ 2022-03-12 20:28 UTC (permalink / raw)
To: Alan Stern, Greg Kroah-Hartman, linux-usb
The UHCI driver's debugging code uses a lot of sprintf() calls with the
large buffers, leaving some space at the end of the buffers to handle the
buffer overflow. Using scnprntf() instead eliminates the very possibility
of the buffer overflow, while simplifying the code at the expense of not
printing an ellipsis when the end of buffer is actually reached...
Found by Linux Verification Center (linuxtesting.org) with the SVACE static
analysis tool.
Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
---
drivers/usb/host/uhci-debug.c | 258 ++++++++++++----------------------
1 file changed, 90 insertions(+), 168 deletions(-)
diff --git a/drivers/usb/host/uhci-debug.c b/drivers/usb/host/uhci-debug.c
index c4e67c4b51f6..b5425dbd3e2d 100644
--- a/drivers/usb/host/uhci-debug.c
+++ b/drivers/usb/host/uhci-debug.c
@@ -43,14 +43,14 @@ static void lprintk(char *buf)
static int uhci_show_td(struct uhci_hcd *uhci, struct uhci_td *td, char *buf,
int len, int space)
{
- char *out = buf;
char *spid;
u32 status, token;
+ int n = 0;
status = td_status(uhci, td);
- out += sprintf(out, "%*s[%p] link (%08x) ", space, "", td,
+ n += scnprintf(buf + n, len - n, "%*s[%p] link (%08x) ", space, "", td,
hc32_to_cpu(uhci, td->link));
- out += sprintf(out, "e%d %s%s%s%s%s%s%s%s%s%sLength=%x ",
+ n += scnprintf(buf + n, len - n, "e%d %s%s%s%s%s%s%s%s%s%sLength=%x ",
((status >> 27) & 3),
(status & TD_CTRL_SPD) ? "SPD " : "",
(status & TD_CTRL_LS) ? "LS " : "",
@@ -63,8 +63,6 @@ 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)) {
@@ -82,38 +80,35 @@ static int uhci_show_td(struct uhci_hcd *uhci, struct uhci_td *td, char *buf,
break;
}
- out += sprintf(out, "MaxLen=%x DT%d EndPt=%x Dev=%x, PID=%x(%s) ",
+ n += scnprintf(buf + n, len - n,
+ "MaxLen=%x DT%d EndPt=%x Dev=%x, PID=%x(%s) ",
token >> 21,
((token >> 19) & 1),
(token >> 15) & 15,
(token >> 8) & 127,
(token & 0xff),
spid);
- out += sprintf(out, "(buf=%08x)\n", hc32_to_cpu(uhci, td->buffer));
+ n += scnprintf(buf + n, len - n, "(buf=%08x)\n",
+ hc32_to_cpu(uhci, td->buffer));
-done:
- if (out - buf > len)
- out += sprintf(out, " ...\n");
- return out - buf;
+ return n;
}
static int uhci_show_urbp(struct uhci_hcd *uhci, struct urb_priv *urbp,
char *buf, int len, int space)
{
- char *out = buf;
struct uhci_td *td;
int i, nactive, ninactive;
char *ptype;
-
-
- out += sprintf(out, "urb_priv [%p] ", urbp);
- out += sprintf(out, "urb [%p] ", urbp->urb);
- out += sprintf(out, "qh [%p] ", urbp->qh);
- out += sprintf(out, "Dev=%d ", usb_pipedevice(urbp->urb->pipe));
- out += sprintf(out, "EP=%x(%s) ", usb_pipeendpoint(urbp->urb->pipe),
+ int n = 0;
+
+ n += scnprintf(buf + n, len - n, "urb_priv [%p] ", urbp);
+ n += scnprintf(buf + n, len - n, "urb [%p] ", urbp->urb);
+ n += scnprintf(buf + n, len - n, "qh [%p] ", urbp->qh);
+ n += scnprintf(buf + n, len - n, "Dev=%d ", usb_pipedevice(urbp->urb->pipe));
+ n += scnprintf(buf + n, len - n, "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;
@@ -123,27 +118,21 @@ static int uhci_show_urbp(struct uhci_hcd *uhci, struct urb_priv *urbp,
case PIPE_CONTROL: ptype = "CTL"; break;
}
- out += sprintf(out, "%s%s", ptype, (urbp->fsbr ? " FSBR" : ""));
- out += sprintf(out, " Actlen=%d%s", urbp->urb->actual_length,
+ n += scnprintf(buf + n, len - n, "%s%s", ptype, (urbp->fsbr ? " FSBR" : ""));
+ n += scnprintf(buf + n, len - n, " Actlen=%d%s", urbp->urb->actual_length,
(urbp->qh->type == USB_ENDPOINT_XFER_CONTROL ?
"-8" : ""));
if (urbp->urb->unlinked)
- out += sprintf(out, " Unlinked=%d", urbp->urb->unlinked);
- out += sprintf(out, "\n");
-
- if (out - buf > len)
- goto done;
+ n += scnprintf(buf + n, len - n, " Unlinked=%d", urbp->urb->unlinked);
+ n += scnprintf(buf + n, len - n, "\n");
i = nactive = ninactive = 0;
list_for_each_entry(td, &urbp->td_list, list) {
if (urbp->qh->type != USB_ENDPOINT_XFER_ISOC &&
(++i <= 10 || debug > 2)) {
- 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;
+ n += scnprintf(buf + n, len - n, "%*s%d: ", space + 2, "", i);
+ n += uhci_show_td(uhci, td, buf + n, len - n, 0);
} else {
if (td_status(uhci, td) & TD_CTRL_ACTIVE)
++nactive;
@@ -152,23 +141,19 @@ static int uhci_show_urbp(struct uhci_hcd *uhci, struct urb_priv *urbp,
}
}
if (nactive + ninactive > 0)
- out += sprintf(out,
+ n += scnprintf(buf + n, len - n,
"%*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;
+ return n;
}
static int uhci_show_qh(struct uhci_hcd *uhci,
struct uhci_qh *qh, char *buf, int len, int space)
{
- char *out = buf;
int i, nurbs;
__hc32 element = qh_element(qh);
char *qtype;
+ int n = 0;
switch (qh->type) {
case USB_ENDPOINT_XFER_ISOC: qtype = "ISO"; break;
@@ -178,43 +163,37 @@ static int uhci_show_qh(struct uhci_hcd *uhci,
default: qtype = "Skel" ; break;
}
- out += sprintf(out, "%*s[%p] %s QH link (%08x) element (%08x)\n",
+ n += scnprintf(buf + n, len - n, "%*s[%p] %s QH link (%08x) element (%08x)\n",
space, "", qh, qtype,
hc32_to_cpu(uhci, qh->link),
hc32_to_cpu(uhci, element));
if (qh->type == USB_ENDPOINT_XFER_ISOC)
- out += sprintf(out,
+ n += scnprintf(buf + n, len - n,
"%*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)
- out += sprintf(out, "%*s period %d phase %d load %d us\n",
+ n += scnprintf(buf + n, len - n, "%*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, "");
+ n += scnprintf(buf + n, len - n, "%*s Element points to QH (bug?)\n",
+ space, "");
if (element & UHCI_PTR_DEPTH(uhci))
- out += sprintf(out, "%*s Depth traverse\n", space, "");
+ n += scnprintf(buf + n, len - n, "%*s Depth traverse\n", space, "");
if (element & cpu_to_hc32(uhci, 8))
- out += sprintf(out, "%*s Bit 3 set (bug?)\n", space, "");
+ n += scnprintf(buf + n, len - n, "%*s Bit 3 set (bug?)\n", space, "");
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;
+ n += scnprintf(buf + n, len - n, "%*s Element is NULL (bug?)\n", space, "");
if (list_empty(&qh->queue)) {
- out += sprintf(out, "%*s queue is empty\n", space, "");
+ n += scnprintf(buf + n, len - n, "%*s queue is empty\n", space, "");
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;
+ n += uhci_show_td(uhci, uhci->term_td, buf + n,
+ len - n, 0);
}
} else {
struct urb_priv *urbp = list_entry(qh->queue.next,
@@ -223,45 +202,32 @@ static int uhci_show_qh(struct uhci_hcd *uhci,
struct uhci_td, list);
if (element != LINK_TO_TD(uhci, td))
- out += sprintf(out, "%*s Element != First TD\n",
+ n += scnprintf(buf + n, len - n, "%*s Element != First TD\n",
space, "");
i = nurbs = 0;
list_for_each_entry(urbp, &qh->queue, node) {
- if (++i <= 10) {
- out += uhci_show_urbp(uhci, urbp, out,
- len - (out - buf), space + 2);
- if (out - buf > len)
- goto tail;
- }
+ if (++i <= 10)
+ n += uhci_show_urbp(uhci, urbp, buf + n,
+ len - n, space + 2);
else
++nurbs;
}
if (nurbs > 0)
- out += sprintf(out, "%*s Skipped %d URBs\n",
+ n += scnprintf(buf + n, len - n, "%*s Skipped %d URBs\n",
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;
+ n += scnprintf(buf + n, len - n, "%*s Dummy TD\n", space, "");
+ n += uhci_show_td(uhci, qh->dummy_td, buf + n, len - n, 0);
}
-done:
- if (out - buf > len)
- out += sprintf(out, " ...\n");
-tail:
- return out - buf;
+ return n;
}
-static int uhci_show_sc(int port, unsigned short status, char *buf)
+static int uhci_show_sc(int port, unsigned short status, char *buf, int len)
{
- return sprintf(buf, " stat%d = %04x %s%s%s%s%s%s%s%s%s%s\n",
+ return scnprintf(buf, len, " stat%d = %04x %s%s%s%s%s%s%s%s%s%s\n",
port,
status,
(status & USBPORTSC_SUSP) ? " Suspend" : "",
@@ -276,7 +242,7 @@ static int uhci_show_sc(int port, unsigned short status, char *buf)
(status & USBPORTSC_CCS) ? " Connected" : "");
}
-static int uhci_show_root_hub_state(struct uhci_hcd *uhci, char *buf)
+static int uhci_show_root_hub_state(struct uhci_hcd *uhci, char *buf, int len)
{
char *rh_state;
@@ -298,18 +264,17 @@ static int uhci_show_root_hub_state(struct uhci_hcd *uhci, char *buf)
default:
rh_state = "?"; break;
}
- return sprintf(buf, "Root-hub state: %s FSBR: %d\n",
+ return scnprintf(buf, len, "Root-hub state: %s FSBR: %d\n",
rh_state, uhci->fsbr_is_on);
}
static int uhci_show_status(struct uhci_hcd *uhci, char *buf, int len)
{
- char *out = buf;
unsigned short usbcmd, usbstat, usbint, usbfrnum;
unsigned int flbaseadd;
unsigned char sof;
unsigned short portsc1, portsc2;
-
+ int n = 0;
usbcmd = uhci_readw(uhci, USBCMD);
usbstat = uhci_readw(uhci, USBSTS);
@@ -320,7 +285,7 @@ static int uhci_show_status(struct uhci_hcd *uhci, char *buf, int len)
portsc1 = uhci_readw(uhci, USBPORTSC1);
portsc2 = uhci_readw(uhci, USBPORTSC2);
- out += sprintf(out, " usbcmd = %04x %s%s%s%s%s%s%s%s\n",
+ n += scnprintf(buf + n, len - n, " usbcmd = %04x %s%s%s%s%s%s%s%s\n",
usbcmd,
(usbcmd & USBCMD_MAXP) ? "Maxp64 " : "Maxp32 ",
(usbcmd & USBCMD_CF) ? "CF " : "",
@@ -330,10 +295,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",
+ n += scnprintf(buf + n, len - n, " usbstat = %04x %s%s%s%s%s%s\n",
usbstat,
(usbstat & USBSTS_HCH) ? "HCHalted " : "",
(usbstat & USBSTS_HCPE) ? "HostControllerProcessError " : "",
@@ -341,39 +304,27 @@ 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);
- 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,
+
+ n += scnprintf(buf + n, len - n, " usbint = %04x\n", usbint);
+ n += scnprintf(buf + n, len - n, " usbfrnum = (%d)%03x\n",
+ (usbfrnum >> 10) & 1, 0xfff & (4 * (unsigned int)usbfrnum));
+ n += scnprintf(buf + n, len - n, " flbaseadd = %08x\n", flbaseadd);
+ n += scnprintf(buf + n, len - n, " sof = %02x\n", sof);
+
+ n += uhci_show_sc(1, portsc1, buf + n, len - n);
+
+ n += uhci_show_sc(2, portsc2, buf + n, len - n);
+
+ n += scnprintf(buf + n, len - n,
"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;
+ return n;
}
static int uhci_sprint_schedule(struct uhci_hcd *uhci, char *buf, int len)
{
- char *out = buf;
int i, j;
struct uhci_qh *qh;
struct uhci_td *td;
@@ -381,41 +332,36 @@ static int uhci_sprint_schedule(struct uhci_hcd *uhci, char *buf, int len)
int nframes, nerrs;
__hc32 link;
__hc32 fsbr_link;
+ int n = 0;
static const char * const qh_names[] = {
"unlink", "iso", "int128", "int64", "int32", "int16",
"int8", "int4", "int2", "async", "term"
};
- 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;
+ n += uhci_show_root_hub_state(uhci, buf + n, len - n);
+ n += scnprintf(buf + n, len - n, "HC status\n");
+ n += uhci_show_status(uhci, buf + n, len - n);
- out += sprintf(out, "Periodic load table\n");
+ n += scnprintf(buf + n, len - n, "Periodic load table\n");
for (i = 0; i < MAX_PHASE; ++i) {
- out += sprintf(out, "\t%d", uhci->load[i]);
+ n += scnprintf(buf + n, len - n, "\t%d", uhci->load[i]);
if (i % 8 == 7)
- *out++ = '\n';
+ buf[n++] = '\n';
}
- out += sprintf(out, "Total: %d, #INT: %d, #ISO: %d\n",
+ n += scnprintf(buf + n, len - n, "Total: %d, #INT: %d, #ISO: %d\n",
uhci->total_load,
uhci_to_hcd(uhci)->self.bandwidth_int_reqs,
uhci_to_hcd(uhci)->self.bandwidth_isoc_reqs);
if (debug <= 1)
goto tail;
- out += sprintf(out, "Frame List\n");
+ n += scnprintf(buf + n, len - n, "Frame List\n");
nframes = 10;
nerrs = 0;
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];
@@ -423,7 +369,7 @@ static int uhci_sprint_schedule(struct uhci_hcd *uhci, char *buf, int len)
goto check_link;
if (nframes > 0) {
- out += sprintf(out, "- Frame %d -> (%08x)\n",
+ n += scnprintf(buf + n, len - n, "- Frame %d -> (%08x)\n",
i, hc32_to_cpu(uhci, link));
j = 1;
}
@@ -435,19 +381,13 @@ 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,
+ n += scnprintf(buf + n, len - n,
" link does not match list entry!\n");
- if (out - buf > len)
- goto done;
} else
++nerrs;
}
- if (nframes > 0) {
- out += uhci_show_td(uhci, td, out,
- len - (out - buf), 4);
- if (out - buf > len)
- goto tail;
- }
+ if (nframes > 0)
+ n += uhci_show_td(uhci, td, buf + n, len - n, 4);
link = td->link;
} while (tmp != head);
@@ -456,46 +396,37 @@ static int uhci_sprint_schedule(struct uhci_hcd *uhci, char *buf, int len)
if (link != qh_dma) {
if (nframes > 0) {
if (!j) {
- out += sprintf(out,
+ n += scnprintf(buf + n, len - n,
"- Frame %d -> (%08x)\n",
i, hc32_to_cpu(uhci, link));
j = 1;
}
- out += sprintf(out,
+ n += scnprintf(buf + n, len - n,
" link does not match QH (%08x)!\n",
hc32_to_cpu(uhci, qh_dma));
- if (out - buf > len)
- goto done;
} else
++nerrs;
}
nframes -= j;
}
if (nerrs > 0)
- out += sprintf(out, "Skipped %d bad links\n", nerrs);
-
- out += sprintf(out, "Skeleton QHs\n");
+ n += scnprintf(buf + n, len - n, "Skipped %d bad links\n", nerrs);
- if (out - buf > len)
- goto done;
+ n += scnprintf(buf + n, len - n, "Skeleton QHs\n");
fsbr_link = 0;
for (i = 0; i < UHCI_NUM_SKELQH; ++i) {
int cnt = 0;
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;
+ n += scnprintf(buf + n, len - n, "- skel_%s_qh\n", qh_names[i]);
+ n += uhci_show_qh(uhci, qh, buf + n, len - n, 4);
/* 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,
+ n += scnprintf(buf + n, len - n,
" skel_term_qh element is not set to term_td!\n");
- if (out - buf > len)
- goto done;
}
link = fsbr_link;
if (!link)
@@ -509,17 +440,14 @@ static int uhci_sprint_schedule(struct uhci_hcd *uhci, char *buf, int len)
while (tmp != head) {
qh = list_entry(tmp, struct uhci_qh, node);
tmp = tmp->next;
- if (++cnt <= 10) {
- out += uhci_show_qh(uhci, qh, out,
- len - (out - buf), 4);
- if (out - buf > len)
- goto tail;
- }
+ if (++cnt <= 10)
+ n += uhci_show_qh(uhci, qh, buf + n, len - n, 4);
if (!fsbr_link && qh->skel >= SKEL_FSBR)
fsbr_link = LINK_TO_QH(uhci, qh);
}
if ((cnt -= 10) > 0)
- out += sprintf(out, " Skipped %d QHs\n", cnt);
+ n += scnprintf(buf + n, len - n, " Skipped %d QHs\n",
+ cnt);
link = UHCI_PTR_TERM(uhci);
if (i <= SKEL_ISO)
@@ -532,18 +460,12 @@ static int uhci_sprint_schedule(struct uhci_hcd *uhci, char *buf, int len)
link = LINK_TO_QH(uhci, uhci->skel_term_qh);
check_qh_link:
if (qh->link != link)
- out += sprintf(out,
+ n += scnprintf(buf + n, len - n,
" 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;
+ return n;
}
#ifdef CONFIG_DEBUG_FS
--
2.26.3
^ permalink raw reply related [flat|nested] 5+ messages in thread