Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1581030 > unrolled thread

[PATCH v2 0/3] staging: fbtft: Fix buffer overflow vulnerability

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 0/3] staging: fbtft: Fix buffer overflow vulnerability "Tobin C. Harding" <me@tobin.cc> - 2017-02-15 04:30 +0100
    [PATCH v2 2/3] staging: fbtft: Replace magic number with constant "Tobin C. Harding" <me@tobin.cc> - 2017-02-15 04:30 +0100
      Re: [PATCH v2 2/3] staging: fbtft: Replace magic number with  constant Joe Perches <joe@perches.com> - 2017-02-15 04:40 +0100
    [PATCH v2 3/3] staging: fbtft: Add check on strlcpy() return value "Tobin C. Harding" <me@tobin.cc> - 2017-02-15 04:30 +0100

#1581030 — [PATCH v2 0/3] staging: fbtft: Fix buffer overflow vulnerability

Module copies a user supplied string (module parameter) into a buffer
using strncpy() and does not check that the buffer is null terminated.

Replace call to strncpy() with call to strlcpy() ensuring that the
buffer is null terminated.

Replace magic number with pre-existing compile time constant.

Check return value of call to strlcpy() and throw warning if source
string is truncated.

v1 was a single patch. v2 adds 2 extra patches while retaining the
original v1 patch as the first of the series.

v2:
 - Replace magic number
 - Check return value of call to strlcpy()

Tobin C. Harding (3):
  staging: fbtft: Fix buffer overflow vulnerability
  staging: fbtft: Replace magic number with constant
  staging: fbtft: Add check on strlcpy() return value

 drivers/staging/fbtft/fbtft_device.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

-- 
2.7.4

[toc] | [next] | [standalone]

#1581031 — [PATCH v2 2/3] staging: fbtft: Replace magic number with constant

Current call to strncmp() uses a magic number. There is a compile
time constant defined for this buffer, included and used already at
other sites in the file.

Remove magic number. Replace with pre-existing compile time constant.

Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
 drivers/staging/fbtft/fbtft_device.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/fbtft/fbtft_device.c b/drivers/staging/fbtft/fbtft_device.c
index 7b7223b..5fbdd37 100644
--- a/drivers/staging/fbtft/fbtft_device.c
+++ b/drivers/staging/fbtft/fbtft_device.c
@@ -1489,7 +1489,7 @@ static int __init fbtft_device_init(void)
 	}
 
 	for (i = 0; i < ARRAY_SIZE(displays); i++) {
-		if (strncmp(name, displays[i].name, 32) == 0) {
+		if (strncmp(name, displays[i].name, SPI_NAME_SIZE) == 0) {
 			if (displays[i].spi) {
 				spi = displays[i].spi;
 				spi->chip_select = cs;
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1581038 — Re: [PATCH v2 2/3] staging: fbtft: Replace magic number with constant

On Wed, 2017-02-15 at 14:27 +1100, Tobin C. Harding wrote:
> Current call to strncmp() uses a magic number. There is a compile
> time constant defined for this buffer, included and used already at
> other sites in the file.
> 
> Remove magic number. Replace with pre-existing compile time constant.

OK thanks, as well:

> diff --git a/drivers/staging/fbtft/fbtft_device.c b/drivers/staging/fbtft/fbtft_device.c
[]
> @@ -1489,7 +1489,7 @@ static int __init fbtft_device_init(void)
>  	}
>  
>  	for (i = 0; i < ARRAY_SIZE(displays); i++) {
> -		if (strncmp(name, displays[i].name, 32) == 0) {
> +		if (strncmp(name, displays[i].name, SPI_NAME_SIZE) == 0) {

Maybe change this to:

		if (strncmp(name, displays[i].name, SPI_NAME_SIZE) != 0)
			continue;

and reduce the indentation of the rest of the block.

[toc] | [prev] | [next] | [standalone]

#1581033 — [PATCH v2 3/3] staging: fbtft: Add check on strlcpy() return value

Return value of strlcpy() is not checked. Name string is silently
truncated if longer that SPI_NAME_SIZE, whilst not detrimental to
the program logic it would be nice to notify the user. Module is
currently quite verbose, adding extra pr_warn() calls will not overly
impact this verbosity.

Check return value from call to strlcpy(). If source string is
truncated call pr_warn() to notify user.

Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
 drivers/staging/fbtft/fbtft_device.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/fbtft/fbtft_device.c b/drivers/staging/fbtft/fbtft_device.c
index 5fbdd37..78baf46 100644
--- a/drivers/staging/fbtft/fbtft_device.c
+++ b/drivers/staging/fbtft/fbtft_device.c
@@ -1483,7 +1483,13 @@ static int __init fbtft_device_init(void)
 			displays[i].pdev->name = name;
 			displays[i].spi = NULL;
 		} else {
-			strlcpy(displays[i].spi->modalias, name, SPI_NAME_SIZE);
+			size_t len;
+
+			len = strlcpy(displays[i].spi->modalias, name,
+				SPI_NAME_SIZE);
+			if (len >= SPI_NAME_SIZE)
+				pr_warn("modalias (name) truncated to: %s\n",
+					displays[i].spi->modalias);
 			displays[i].pdev = NULL;
 		}
 	}
-- 
2.7.4

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web