Skip to content

Commit

Permalink
usb: yurex: Replace snprintf() with the safer scnprintf() variant
Browse files Browse the repository at this point in the history
[ Upstream commit 86b20af11e84c26ae3fde4dcc4f490948e3f8035 ]

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.

Whilst we're at it, let's define some magic numbers to increase
readability and ease of maintenance.

Link: https://lwn.net/Articles/69419/
Link: KSPP/linux#105
Cc: Tomoki Sekiyama <tomoki.sekiyama@gmail.com>
Signed-off-by: Lee Jones <lee@kernel.org>
Link: https://lore.kernel.org/r/20231213164246.1021885-9-lee@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Stable-dep-of: 93907620b308 ("USB: misc: yurex: fix race between read and write")
Signed-off-by: Sasha Levin <sashal@kernel.org>
(cherry picked from commit a2ac6cb8aaa2eb23209ffa641962dd62958522a1)
Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
  • Loading branch information
lag-linaro authored and AzyrRuthless committed Feb 18, 2025
1 parent 257831b commit 145d988
Showing 1 changed file with 7 additions and 5 deletions.
12 changes: 7 additions & 5 deletions drivers/usb/misc/yurex.c
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@
#define YUREX_BUF_SIZE 8
#define YUREX_WRITE_TIMEOUT (HZ*2)

#define MAX_S64_STRLEN 20 /* {-}922337203685477580{7,8} */

/* table of devices that work with this driver */
static struct usb_device_id yurex_table[] = {
{ USB_DEVICE(YUREX_VENDOR_ID, YUREX_PRODUCT_ID) },
Expand Down Expand Up @@ -406,7 +408,7 @@ static ssize_t yurex_read(struct file *file, char __user *buffer, size_t count,
{
struct usb_yurex *dev;
int len = 0;
char in_buffer[20];
char in_buffer[MAX_S64_STRLEN];
unsigned long flags;

dev = file->private_data;
Expand All @@ -417,14 +419,14 @@ static ssize_t yurex_read(struct file *file, char __user *buffer, size_t count,
return -ENODEV;
}

if (WARN_ON_ONCE(dev->bbu > S64_MAX || dev->bbu < S64_MIN))
return -EIO;

spin_lock_irqsave(&dev->lock, flags);
len = snprintf(in_buffer, 20, "%lld\n", dev->bbu);
scnprintf(in_buffer, MAX_S64_STRLEN, "%lld\n", dev->bbu);
spin_unlock_irqrestore(&dev->lock, flags);
mutex_unlock(&dev->io_mutex);

if (WARN_ON_ONCE(len >= sizeof(in_buffer)))
return -EIO;

return simple_read_from_buffer(buffer, count, ppos, in_buffer, len);
}

Expand Down

0 comments on commit 145d988

Please sign in to comment.