From: Lee Jones <lee@kernel.org>
To: lee@kernel.org, gregkh@linuxfoundation.org
Cc: linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org,
Pawel Laszczak <pawell@cadence.com>
Subject: [PATCH 06/12] usb: cdns2: Replace snprintf() with the safer scnprintf() variant
Date: Wed, 13 Dec 2023 16:42:35 +0000 [thread overview]
Message-ID: <20231213164246.1021885-7-lee@kernel.org> (raw)
In-Reply-To: <20231213164246.1021885-1-lee@kernel.org>
There is a general misunderstanding amongst engineers that {v}snprintf()
returns the length of the data *actually* encoded into the destination
array. However, as per the C99 standard {v}snprintf() really returns
the length of the data that *would have been* written if there were
enough space for it. This misunderstanding has led to buffer-overruns
in the past. It's generally considered safer to use the {v}scnprintf()
variants in their place (or even sprintf() in simple cases). So let's
do that.
Link: https://lwn.net/Articles/69419/
Link: https://github.com/KSPP/linux/issues/105
Cc: Pawel Laszczak <pawell@cadence.com>
Signed-off-by: Lee Jones <lee@kernel.org>
---
drivers/usb/gadget/udc/cdns2/cdns2-debug.h | 138 ++++++++++-----------
1 file changed, 69 insertions(+), 69 deletions(-)
diff --git a/drivers/usb/gadget/udc/cdns2/cdns2-debug.h b/drivers/usb/gadget/udc/cdns2/cdns2-debug.h
index be9ae0d28a409..f5f330004190e 100644
--- a/drivers/usb/gadget/udc/cdns2/cdns2-debug.h
+++ b/drivers/usb/gadget/udc/cdns2/cdns2-debug.h
@@ -16,34 +16,34 @@ static inline const char *cdns2_decode_usb_irq(char *str, size_t size,
{
int ret;
- ret = snprintf(str, size, "usbirq: 0x%02x - ", usb_irq);
+ ret = scnprintf(str, size, "usbirq: 0x%02x - ", usb_irq);
if (usb_irq & USBIRQ_SOF)
- ret += snprintf(str + ret, size - ret, "SOF ");
+ ret += scnprintf(str + ret, size - ret, "SOF ");
if (usb_irq & USBIRQ_SUTOK)
- ret += snprintf(str + ret, size - ret, "SUTOK ");
+ ret += scnprintf(str + ret, size - ret, "SUTOK ");
if (usb_irq & USBIRQ_SUDAV)
- ret += snprintf(str + ret, size - ret, "SETUP ");
+ ret += scnprintf(str + ret, size - ret, "SETUP ");
if (usb_irq & USBIRQ_SUSPEND)
- ret += snprintf(str + ret, size - ret, "Suspend ");
+ ret += scnprintf(str + ret, size - ret, "Suspend ");
if (usb_irq & USBIRQ_URESET)
- ret += snprintf(str + ret, size - ret, "Reset ");
+ ret += scnprintf(str + ret, size - ret, "Reset ");
if (usb_irq & USBIRQ_HSPEED)
- ret += snprintf(str + ret, size - ret, "HS ");
+ ret += scnprintf(str + ret, size - ret, "HS ");
if (usb_irq & USBIRQ_LPM)
- ret += snprintf(str + ret, size - ret, "LPM ");
+ ret += scnprintf(str + ret, size - ret, "LPM ");
- ret += snprintf(str + ret, size - ret, ", EXT: 0x%02x - ", ext_irq);
+ ret += scnprintf(str + ret, size - ret, ", EXT: 0x%02x - ", ext_irq);
if (ext_irq & EXTIRQ_WAKEUP)
- ret += snprintf(str + ret, size - ret, "Wakeup ");
+ ret += scnprintf(str + ret, size - ret, "Wakeup ");
if (ext_irq & EXTIRQ_VBUSFAULT_FALL)
- ret += snprintf(str + ret, size - ret, "VBUS_FALL ");
+ ret += scnprintf(str + ret, size - ret, "VBUS_FALL ");
if (ext_irq & EXTIRQ_VBUSFAULT_RISE)
- ret += snprintf(str + ret, size - ret, "VBUS_RISE ");
+ ret += scnprintf(str + ret, size - ret, "VBUS_RISE ");
- if (ret >= size)
- pr_info("CDNS2: buffer overflowed.\n");
+ if (ret == size - 1)
+ pr_info("CDNS2: buffer may be truncated.\n");
return str;
}
@@ -54,28 +54,28 @@ static inline const char *cdns2_decode_dma_irq(char *str, size_t size,
{
int ret;
- ret = snprintf(str, size, "ISTS: %08x, %s: %08x ",
- ep_ists, ep_name, ep_sts);
+ ret = scnprintf(str, size, "ISTS: %08x, %s: %08x ",
+ ep_ists, ep_name, ep_sts);
if (ep_sts & DMA_EP_STS_IOC)
- ret += snprintf(str + ret, size - ret, "IOC ");
+ ret += scnprintf(str + ret, size - ret, "IOC ");
if (ep_sts & DMA_EP_STS_ISP)
- ret += snprintf(str + ret, size - ret, "ISP ");
+ ret += scnprintf(str + ret, size - ret, "ISP ");
if (ep_sts & DMA_EP_STS_DESCMIS)
- ret += snprintf(str + ret, size - ret, "DESCMIS ");
+ ret += scnprintf(str + ret, size - ret, "DESCMIS ");
if (ep_sts & DMA_EP_STS_TRBERR)
- ret += snprintf(str + ret, size - ret, "TRBERR ");
+ ret += scnprintf(str + ret, size - ret, "TRBERR ");
if (ep_sts & DMA_EP_STS_OUTSMM)
- ret += snprintf(str + ret, size - ret, "OUTSMM ");
+ ret += scnprintf(str + ret, size - ret, "OUTSMM ");
if (ep_sts & DMA_EP_STS_ISOERR)
- ret += snprintf(str + ret, size - ret, "ISOERR ");
+ ret += scnprintf(str + ret, size - ret, "ISOERR ");
if (ep_sts & DMA_EP_STS_DBUSY)
- ret += snprintf(str + ret, size - ret, "DBUSY ");
+ ret += scnprintf(str + ret, size - ret, "DBUSY ");
if (DMA_EP_STS_CCS(ep_sts))
- ret += snprintf(str + ret, size - ret, "CCS ");
+ ret += scnprintf(str + ret, size - ret, "CCS ");
- if (ret >= size)
- pr_info("CDNS2: buffer overflowed.\n");
+ if (ret == size - 1)
+ pr_info("CDNS2: buffer may be truncated.\n");
return str;
}
@@ -105,43 +105,43 @@ static inline const char *cdns2_raw_ring(struct cdns2_endpoint *pep,
int ret;
int i;
- ret = snprintf(str, size, "\n\t\tTR for %s:", pep->name);
+ ret = scnprintf(str, size, "\n\t\tTR for %s:", pep->name);
trb = &trbs[ring->dequeue];
dma = cdns2_trb_virt_to_dma(pep, trb);
- ret += snprintf(str + ret, size - ret,
- "\n\t\tRing deq index: %d, trb: V=%p, P=0x%pad\n",
- ring->dequeue, trb, &dma);
+ ret += scnprintf(str + ret, size - ret,
+ "\n\t\tRing deq index: %d, trb: V=%p, P=0x%pad\n",
+ ring->dequeue, trb, &dma);
trb = &trbs[ring->enqueue];
dma = cdns2_trb_virt_to_dma(pep, trb);
- ret += snprintf(str + ret, size - ret,
- "\t\tRing enq index: %d, trb: V=%p, P=0x%pad\n",
- ring->enqueue, trb, &dma);
+ ret += scnprintf(str + ret, size - ret,
+ "\t\tRing enq index: %d, trb: V=%p, P=0x%pad\n",
+ ring->enqueue, trb, &dma);
- ret += snprintf(str + ret, size - ret,
- "\t\tfree trbs: %d, CCS=%d, PCS=%d\n",
- ring->free_trbs, ring->ccs, ring->pcs);
+ ret += scnprintf(str + ret, size - ret,
+ "\t\tfree trbs: %d, CCS=%d, PCS=%d\n",
+ ring->free_trbs, ring->ccs, ring->pcs);
if (TRBS_PER_SEGMENT > 40) {
- ret += snprintf(str + ret, size - ret,
- "\t\tTransfer ring %d too big\n", TRBS_PER_SEGMENT);
+ ret += scnprintf(str + ret, size - ret,
+ "\t\tTransfer ring %d too big\n", TRBS_PER_SEGMENT);
return str;
}
dma = ring->dma;
for (i = 0; i < TRBS_PER_SEGMENT; ++i) {
trb = &trbs[i];
- ret += snprintf(str + ret, size - ret,
- "\t\t@%pad %08x %08x %08x\n", &dma,
- le32_to_cpu(trb->buffer),
- le32_to_cpu(trb->length),
- le32_to_cpu(trb->control));
+ ret += scnprintf(str + ret, size - ret,
+ "\t\t@%pad %08x %08x %08x\n", &dma,
+ le32_to_cpu(trb->buffer),
+ le32_to_cpu(trb->length),
+ le32_to_cpu(trb->control));
dma += sizeof(*trb);
}
- if (ret >= size)
- pr_info("CDNS2: buffer overflowed.\n");
+ if (ret == size - 1)
+ pr_info("CDNS2: buffer may be truncated.\n");
return str;
}
@@ -166,36 +166,36 @@ static inline const char *cdns2_decode_trb(char *str, size_t size, u32 flags,
switch (type) {
case TRB_LINK:
- ret = snprintf(str, size,
- "LINK %08x type '%s' flags %c:%c:%c%c:%c",
- buffer, cdns2_trb_type_string(type),
- flags & TRB_CYCLE ? 'C' : 'c',
- flags & TRB_TOGGLE ? 'T' : 't',
- flags & TRB_CHAIN ? 'C' : 'c',
- flags & TRB_CHAIN ? 'H' : 'h',
- flags & TRB_IOC ? 'I' : 'i');
+ ret = scnprintf(str, size,
+ "LINK %08x type '%s' flags %c:%c:%c%c:%c",
+ buffer, cdns2_trb_type_string(type),
+ flags & TRB_CYCLE ? 'C' : 'c',
+ flags & TRB_TOGGLE ? 'T' : 't',
+ flags & TRB_CHAIN ? 'C' : 'c',
+ flags & TRB_CHAIN ? 'H' : 'h',
+ flags & TRB_IOC ? 'I' : 'i');
break;
case TRB_NORMAL:
- ret = snprintf(str, size,
- "type: '%s', Buffer: %08x, length: %ld, burst len: %ld, "
- "flags %c:%c:%c%c:%c",
- cdns2_trb_type_string(type),
- buffer, TRB_LEN(length),
- TRB_FIELD_TO_BURST(length),
- flags & TRB_CYCLE ? 'C' : 'c',
- flags & TRB_ISP ? 'I' : 'i',
- flags & TRB_CHAIN ? 'C' : 'c',
- flags & TRB_CHAIN ? 'H' : 'h',
- flags & TRB_IOC ? 'I' : 'i');
+ ret = scnprintf(str, size,
+ "type: '%s', Buffer: %08x, length: %ld, burst len: %ld, "
+ "flags %c:%c:%c%c:%c",
+ cdns2_trb_type_string(type),
+ buffer, TRB_LEN(length),
+ TRB_FIELD_TO_BURST(length),
+ flags & TRB_CYCLE ? 'C' : 'c',
+ flags & TRB_ISP ? 'I' : 'i',
+ flags & TRB_CHAIN ? 'C' : 'c',
+ flags & TRB_CHAIN ? 'H' : 'h',
+ flags & TRB_IOC ? 'I' : 'i');
break;
default:
- ret = snprintf(str, size, "type '%s' -> raw %08x %08x %08x",
- cdns2_trb_type_string(type),
- buffer, length, flags);
+ ret = scnprintf(str, size, "type '%s' -> raw %08x %08x %08x",
+ cdns2_trb_type_string(type),
+ buffer, length, flags);
}
- if (ret >= size)
- pr_info("CDNS2: buffer overflowed.\n");
+ if (ret == size - 1)
+ pr_info("CDNS2: buffer may be truncated.\n");
return str;
}
--
2.43.0.472.g3155946c3a-goog
next prev parent reply other threads:[~2023-12-13 16:43 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-13 16:42 [PATCH 00/12] usb: Replace {v}snprintf() variants with safer alternatives Lee Jones
2023-12-13 16:42 ` Lee Jones
2023-12-13 16:42 ` [PATCH 01/12] usb: gadget: configfs: Replace snprintf() with the safer scnprintf() variant Lee Jones
2023-12-13 16:42 ` [PATCH 02/12] usb: gadget: f_uac1: " Lee Jones
2023-12-13 16:42 ` [PATCH 03/12] usb: gadget: f_uac2: " Lee Jones
2023-12-13 16:42 ` [PATCH 04/12] usb: gadget: uvc: " Lee Jones
2023-12-13 16:47 ` Laurent Pinchart
2023-12-13 16:42 ` [PATCH 05/12] usb: gadget: udc: atmel: " Lee Jones
2023-12-13 16:42 ` Lee Jones
2023-12-13 16:42 ` Lee Jones [this message]
2023-12-13 16:42 ` [PATCH 07/12] usb: host: max3421-hcd: " Lee Jones
2023-12-13 16:42 ` Lee Jones
2023-12-13 16:42 ` [PATCH 08/12] usb: yurex: " Lee Jones
2023-12-13 16:42 ` [PATCH 09/12] usb: mon_stat: " Lee Jones
2023-12-13 16:42 ` [PATCH 10/12] usb: mon_text: " Lee Jones
2023-12-13 16:42 ` [PATCH 11/12] usb: phy: twl6030: Remove snprintf() from sysfs call-backs and replace with sysfs_emit() Lee Jones
2023-12-13 16:42 ` [PATCH 12/12] usb: storage: " Lee Jones
2023-12-13 18:15 ` Alan Stern
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20231213164246.1021885-7-lee@kernel.org \
--to=lee@kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=pawell@cadence.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.