Skip to contentSkip to conversation

v4l: xilinx: harmless buffer overflow

PATCH In review Dan Carpenter 2 participants 2 messages Apr 21, 2015

Subsystems (4)
Media (V4L/DVB) Kernel janitors Arm/Zynq architecture Xilinx video IP cores
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 1/1 v4l: xilinx: harmless buffer overflow

My static checker warns that the name of the port can be 15 characters when you consider the NUL terminator and that's one more than the 14 characters in name[]. Maybe it's an off-by-one?

It's unlikely that we hit the limit and even if we do the overflow will only affect one of the two bytes of padding so it's harmless. Still let's fix it and also change the sprintf() to snprintf().

Signed-off-by Dan Carpenter <[email protected]>
drivers/media/platform/xilinx/xilinx-dma.c +2 -2
index efde88a..98e50e4 100644
@@ -653,7 +653,7 @@ static const struct v4l2_file_operations xvip_dma_fops = {
 int xvip_dma_init(struct xvip_composite_device *xdev, struct xvip_dma *dma,
 		  enum v4l2_buf_type type, unsigned int port)
 {
-	char name[14];
+	char name[16];
 	int ret;
 
 	dma->xdev = xdev;

@@ -725,7 +725,7 @@ int xvip_dma_init(struct xvip_composite_device *xdev, struct xvip_dma *dma,
 	}
 
 	/* ... and the DMA channel. */
-	sprintf(name, "port%u", port);
+	snprintf(name, sizeof(name), "port%u", port);
 	dma->dma = dma_request_slave_channel(dma->xdev->dev, name);
 	if (dma->dma = NULL) {
 		dev_err(dma->xdev->dev, "no VDMA channel found\n");
maintainer Acked-by on v1 1/1
#1 lore raw

Hi Dan,

Thank you for the patch.

Quoted 18 lines · On Tuesday 21 April 2015 12:31:10 Dan Carpenter wrote:

My static checker warns that the name of the port can be 15 characters when you consider the NUL terminator and that's one more than the 14 characters in name[]. Maybe it's an off-by-one?

It's unlikely that we hit the limit and even if we do the overflow will only affect one of the two bytes of padding so it's harmless. Still let's fix it and also change the sprintf() to snprintf().

Signed-off-by Dan Carpenter <[email protected]>
(quoted diff) +0 -0

b/drivers/media/platform/xilinx/xilinx-dma.c index efde88a..98e50e4 100644

drivers/media/platform/xilinx/xilinx-dma.c +0 -0
@@ -653,7 +653,7 @@ static const struct v4l2_file_operations xvip_dma_fops =

{ int xvip_dma_init(struct xvip_composite_device *xdev, struct xvip_dma *dma, enum v4l2_buf_type type, unsigned int port)
{

drivers/media/platform/xilinx/xilinx-dma.c static const struct v4l2_file_operations xvip_dma_fops = { · line 656

Quoted 2 lines
drivers/media/platform/xilinx/xilinx-dma.c +1 -1
-	char name[14];
+	char name[16];

Being pedantic we could use name[15], but it wouldn't make any difference.

drivers/media/platform/xilinx/xilinx-dma.c int xvip_dma_init(struct xvip_composite_device *xdev, · line 728

Quoted 9 lines
drivers/media/platform/xilinx/xilinx-dma.c +1 -1
 	int ret;
 
 	dma->xdev = xdev;

@@ -725,7 +725,7 @@ int xvip_dma_init(struct xvip_composite_device *xdev,
 struct xvip_dma *dma, }
 
 	/* ... and the DMA channel. */
-	sprintf(name, "port%u", port);
+	snprintf(name, sizeof(name), "port%u", port);

Nitpicking again, I'd say that sprintf is enough as we know it won't overflow. However, as the sprintf implementation is a wrapper around vsnprintf with size set to INT_MAX, using snprintf won't incur any runtime performance penalty.

Acked-by Laurent Pinchart <[email protected]>

and applied to my tree.

Quoted 3 lines
drivers/media/platform/xilinx/xilinx-dma.c +0 -0
 	dma->dma = dma_request_slave_channel(dma->xdev->dev, name);
 	if (dma->dma == NULL) {
 		dev_err(dma->xdev->dev, "no VDMA channel found\n");
Signature · 3 lines
Regards,

Laurent Pinchart