Linux-Modules Archive on lore.kernel.org
 help / Atom feed
* [PATCH 1/3] testsuite: include stdio.h
@ 2016-08-10 18:37 Lucas De Marchi
  2016-08-10 18:37 ` [PATCH 2/3] Add scratchbuf implementation Lucas De Marchi
  2016-08-10 18:37 ` [PATCH 3/3] depmod: fix string overflow Lucas De Marchi
  0 siblings, 2 replies; 5+ messages in thread
From: Lucas De Marchi @ 2016-08-10 18:37 UTC (permalink / raw)
  To: linux-modules; +Cc: Lucas De Marchi

From: Lucas De Marchi <lucas.demarchi@intel.com>

It's used in the log macros so include it.
---
 testsuite/testsuite.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/testsuite/testsuite.h b/testsuite/testsuite.h
index 3bd74f3..bb0eb50 100644
--- a/testsuite/testsuite.h
+++ b/testsuite/testsuite.h
@@ -19,6 +19,7 @@
 
 #include <stdbool.h>
 #include <stdarg.h>
+#include <stdio.h>
 
 #include <shared/macro.h>
 
-- 
2.7.4

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 2/3] Add scratchbuf implementation
  2016-08-10 18:37 [PATCH 1/3] testsuite: include stdio.h Lucas De Marchi
@ 2016-08-10 18:37 ` Lucas De Marchi
  2016-08-10 18:37 ` [PATCH 3/3] depmod: fix string overflow Lucas De Marchi
  1 sibling, 0 replies; 5+ messages in thread
From: Lucas De Marchi @ 2016-08-10 18:37 UTC (permalink / raw)
  To: linux-modules; +Cc: Lucas De Marchi

From: Lucas De Marchi <lucas.demarchi@intel.com>

This should fill the requirements for "we need to loop over a lot of
strings that usually are small enough to remain on stack, but we want to
protect ourselves against huge strings not fitting in the static
buffer we estimated as sufficient"
---
 Makefile.am                 |  5 +++
 shared/scratchbuf.c         | 60 ++++++++++++++++++++++++++++++
 shared/scratchbuf.h         | 23 ++++++++++++
 testsuite/.gitignore        |  3 ++
 testsuite/test-scratchbuf.c | 89 +++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 180 insertions(+)
 create mode 100644 shared/scratchbuf.c
 create mode 100644 shared/scratchbuf.h
 create mode 100644 testsuite/test-scratchbuf.c

diff --git a/Makefile.am b/Makefile.am
index 390628a..d4eeb7e 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -52,6 +52,7 @@ shared_libshared_la_SOURCES = \
 	shared/array.h \
 	shared/hash.c \
 	shared/hash.h \
+	shared/scratchbuf.c \
 	shared/strbuf.c \
 	shared/strbuf.h \
 	shared/util.c \
@@ -322,6 +323,7 @@ testsuite_libtestsuite_la_LIBADD = -lrt
 TESTSUITE = \
 	testsuite/test-hash \
 	testsuite/test-array \
+	testsuite/test-scratchbuf \
 	testsuite/test-strbuf \
 	testsuite/test-init \
 	testsuite/test-initstate \
@@ -349,6 +351,9 @@ testsuite_test_hash_CPPFLAGS = $(TESTSUITE_CPPFLAGS)
 testsuite_test_array_LDADD = $(TESTSUITE_LDADD)
 testsuite_test_array_CPPFLAGS = $(TESTSUITE_CPPFLAGS)
 
+testsuite_test_scratchbuf_LDADD = $(TESTSUITE_LDADD)
+testsuite_test_scratchbuf_CPPFLAGS = $(TESTSUITE_CPPFLAGS)
+
 testsuite_test_strbuf_LDADD = $(TESTSUITE_LDADD)
 testsuite_test_strbuf_CPPFLAGS = $(TESTSUITE_CPPFLAGS)
 
diff --git a/shared/scratchbuf.c b/shared/scratchbuf.c
new file mode 100644
index 0000000..8d9eb83
--- /dev/null
+++ b/shared/scratchbuf.c
@@ -0,0 +1,60 @@
+/*
+ * kmod - interface to kernel module operations
+ *
+ * Copyright (C) 2016  Intel Corporation. All rights reserved.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+#include "scratchbuf.h"
+
+#include <errno.h>
+#include <string.h>
+
+void scratchbuf_init(struct scratchbuf *buf, char *stackbuf, size_t size)
+{
+	buf->bytes = stackbuf;
+	buf->size = size;
+	buf->need_free = false;
+}
+
+int scratchbuf_alloc(struct scratchbuf *buf, size_t size)
+{
+	char *tmp;
+
+	if (size <= buf->size)
+		return 0;
+
+	if (buf->need_free) {
+		tmp = realloc(buf->bytes, size);
+		if (tmp == NULL)
+			return -ENOMEM;
+	} else {
+		tmp = malloc(size);
+		if (tmp == NULL)
+			return -ENOMEM;
+		memcpy(tmp, buf->bytes, buf->size);
+	}
+
+	buf->size = size;
+	buf->bytes = tmp;
+	buf->need_free = true;
+
+	return 0;
+}
+
+void scratchbuf_release(struct scratchbuf *buf)
+{
+	if (buf->need_free)
+		free(buf->bytes);
+}
diff --git a/shared/scratchbuf.h b/shared/scratchbuf.h
new file mode 100644
index 0000000..d14fd14
--- /dev/null
+++ b/shared/scratchbuf.h
@@ -0,0 +1,23 @@
+#pragma once
+
+#include <stdbool.h>
+#include <stdlib.h>
+
+/*
+ * Buffer abstract data type
+ */
+struct scratchbuf {
+	char *bytes;
+	size_t size;
+	bool need_free;
+};
+
+void scratchbuf_init(struct scratchbuf *buf, char *stackbuf, size_t size);
+int scratchbuf_alloc(struct scratchbuf *buf, size_t sz);
+void scratchbuf_release(struct scratchbuf *buf);
+
+/* Return a C string */
+inline char *scratchbuf_str(struct scratchbuf *buf)
+{
+	return buf->bytes;
+}
diff --git a/testsuite/.gitignore b/testsuite/.gitignore
index 2b71a47..9d26b88 100644
--- a/testsuite/.gitignore
+++ b/testsuite/.gitignore
@@ -2,6 +2,7 @@
 *.la
 *.so
 /.dirstamp
+/test-scratchbuf
 /test-strbuf
 /test-array
 /test-util
@@ -20,6 +21,8 @@
 /test-tools
 /rootfs
 /stamp-rootfs
+/test-scratchbuf.log
+/test-scratchbuf.trs
 /test-strbuf.log
 /test-strbuf.trs
 /test-array.log
diff --git a/testsuite/test-scratchbuf.c b/testsuite/test-scratchbuf.c
new file mode 100644
index 0000000..6d86957
--- /dev/null
+++ b/testsuite/test-scratchbuf.c
@@ -0,0 +1,89 @@
+/*
+ * Copyright (C)  2016 Intel Corporation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <errno.h>
+#include <string.h>
+#include <unistd.h>
+
+#include <shared/scratchbuf.h>
+
+#include "testsuite.h"
+
+static int test_scratchbuf_onlystack(const struct test *t)
+{
+	struct scratchbuf sbuf;
+	const char *smallstr = "xyz";
+	char buf[3 + 2];
+	char buf2[3 + 1];
+
+	scratchbuf_init(&sbuf, buf, sizeof(buf));
+	assert_return(scratchbuf_alloc(&sbuf, strlen(smallstr) + 1) == 0, EXIT_FAILURE);
+	assert_return(sbuf.need_free == false, EXIT_FAILURE);
+	scratchbuf_release(&sbuf);
+
+	scratchbuf_init(&sbuf, buf2, sizeof(buf2));
+	assert_return(scratchbuf_alloc(&sbuf, strlen(smallstr) + 1) == 0, EXIT_FAILURE);
+	assert_return(sbuf.need_free == false, EXIT_FAILURE);
+	scratchbuf_release(&sbuf);
+
+	memcpy(scratchbuf_str(&sbuf), smallstr, strlen(smallstr) + 1);
+	assert_return(strcmp(scratchbuf_str(&sbuf), smallstr) == 0, EXIT_FAILURE);
+
+	return 0;
+}
+DEFINE_TEST(test_scratchbuf_onlystack,
+		.description = "test scratchbuf for buffer on stack only");
+
+
+static int test_scratchbuf_heap(const struct test *t)
+{
+	struct scratchbuf sbuf;
+	const char *smallstr = "xyz";
+	const char *largestr = "xyzxyzxyz";
+	const char *largestr2 = "xyzxyzxyzxyzxyz";
+	char buf[3 + 1];
+
+	scratchbuf_init(&sbuf, buf, sizeof(buf));
+
+	/* Initially only on stack */
+	assert_return(scratchbuf_alloc(&sbuf, strlen(smallstr) + 1) == 0, EXIT_FAILURE);
+	assert_return(sbuf.need_free == false, EXIT_FAILURE);
+	memcpy(scratchbuf_str(&sbuf), smallstr, strlen(smallstr) + 1);
+
+	/* Grow once to heap */
+	assert_return(scratchbuf_alloc(&sbuf, strlen(largestr) + 1) == 0, EXIT_FAILURE);
+	assert_return(sbuf.need_free == true, EXIT_FAILURE);
+	assert_return(sbuf.size == strlen(largestr) + 1, EXIT_FAILURE);
+	assert_return(strcmp(scratchbuf_str(&sbuf), smallstr) == 0, EXIT_FAILURE);
+	memcpy(scratchbuf_str(&sbuf), largestr, strlen(largestr) + 1);
+	assert_return(strcmp(scratchbuf_str(&sbuf), largestr) == 0, EXIT_FAILURE);
+
+	/* Grow again - realloc should take place */
+	assert_return(scratchbuf_alloc(&sbuf, strlen(largestr2) + 1) == 0, EXIT_FAILURE);
+	assert_return(sbuf.need_free == true, EXIT_FAILURE);
+	assert_return(sbuf.size == strlen(largestr2) + 1, EXIT_FAILURE);
+	memcpy(scratchbuf_str(&sbuf), largestr2, strlen(largestr2) + 1);
+	assert_return(strcmp(scratchbuf_str(&sbuf), largestr2) == 0, EXIT_FAILURE);
+
+	scratchbuf_release(&sbuf);
+
+	return 0;
+}
+DEFINE_TEST(test_scratchbuf_heap,
+		.description = "test scratchbuf for buffer on that grows to heap");
+
+TESTSUITE_MAIN();
-- 
2.7.4

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 3/3] depmod: fix string overflow
  2016-08-10 18:37 [PATCH 1/3] testsuite: include stdio.h Lucas De Marchi
  2016-08-10 18:37 ` [PATCH 2/3] Add scratchbuf implementation Lucas De Marchi
@ 2016-08-10 18:37 ` Lucas De Marchi
  2016-08-13 20:31   ` Lucas De Marchi
  1 sibling, 1 reply; 5+ messages in thread
From: Lucas De Marchi @ 2016-08-10 18:37 UTC (permalink / raw)
  To: linux-modules; +Cc: Lucas De Marchi

From: Lucas De Marchi <lucas.demarchi@intel.com>

Use scratchbuf to fix issue with strcpy that may overflow the buffer we
declared in the stack.
---
 tools/depmod.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/tools/depmod.c b/tools/depmod.c
index a2e07c1..be9e001 100644
--- a/tools/depmod.c
+++ b/tools/depmod.c
@@ -35,6 +35,7 @@
 #include <shared/hash.h>
 #include <shared/macro.h>
 #include <shared/util.h>
+#include <shared/scratchbuf.h>
 
 #include <libkmod/libkmod.h>
 
@@ -1920,6 +1921,7 @@ static int output_symbols_bin(struct depmod *depmod, FILE *out)
 {
 	struct index_node *idx;
 	char alias[1024];
+	struct scratchbuf salias;
 	size_t baselen = sizeof("symbol:") - 1;
 	struct hash_iter iter;
 	const void *v;
@@ -1932,16 +1934,21 @@ static int output_symbols_bin(struct depmod *depmod, FILE *out)
 		return -ENOMEM;
 
 	memcpy(alias, "symbol:", baselen);
+	scratchbuf_init(&salias, alias, sizeof(alias));
+
 	hash_iter_init(depmod->symbols, &iter);
 
 	while (hash_iter_next(&iter, NULL, &v)) {
 		int duplicate;
 		const struct symbol *sym = v;
+		size_t len;
 
 		if (sym->owner == NULL)
 			continue;
 
-		strcpy(alias + baselen, sym->name);
+		len = strlen(sym->name);
+		scratchbuf_alloc(&salias, baselen + len + 1);
+		memcpy(scratchbuf_str(&salias) + baselen, sym->name, len + 1);
 		duplicate = index_insert(idx, alias, sym->owner->modname,
 							sym->owner->idx);
 
@@ -1951,6 +1958,7 @@ static int output_symbols_bin(struct depmod *depmod, FILE *out)
 	}
 
 	index_write(idx, out);
+	scratchbuf_release(&salias);
 	index_destroy(idx);
 
 	return 0;
-- 
2.7.4

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 3/3] depmod: fix string overflow
  2016-08-10 18:37 ` [PATCH 3/3] depmod: fix string overflow Lucas De Marchi
@ 2016-08-13 20:31   ` Lucas De Marchi
  2016-08-15 13:28     ` Lucas De Marchi
  0 siblings, 1 reply; 5+ messages in thread
From: Lucas De Marchi @ 2016-08-13 20:31 UTC (permalink / raw)
  To: linux-modules; +Cc: Lucas De Marchi

On Wed, Aug 10, 2016 at 3:37 PM, Lucas De Marchi
<lucas.de.marchi@gmail.com> wrote:
> From: Lucas De Marchi <lucas.demarchi@intel.com>
>
> Use scratchbuf to fix issue with strcpy that may overflow the buffer we
> declared in the stack.
> ---
>  tools/depmod.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/tools/depmod.c b/tools/depmod.c
> index a2e07c1..be9e001 100644
> --- a/tools/depmod.c
> +++ b/tools/depmod.c
> @@ -35,6 +35,7 @@
>  #include <shared/hash.h>
>  #include <shared/macro.h>
>  #include <shared/util.h>
> +#include <shared/scratchbuf.h>
>
>  #include <libkmod/libkmod.h>
>
> @@ -1920,6 +1921,7 @@ static int output_symbols_bin(struct depmod *depmod, FILE *out)
>  {
>         struct index_node *idx;
>         char alias[1024];
> +       struct scratchbuf salias;
>         size_t baselen = sizeof("symbol:") - 1;
>         struct hash_iter iter;
>         const void *v;
> @@ -1932,16 +1934,21 @@ static int output_symbols_bin(struct depmod *depmod, FILE *out)
>                 return -ENOMEM;
>
>         memcpy(alias, "symbol:", baselen);
> +       scratchbuf_init(&salias, alias, sizeof(alias));
> +
>         hash_iter_init(depmod->symbols, &iter);
>
>         while (hash_iter_next(&iter, NULL, &v)) {
>                 int duplicate;
>                 const struct symbol *sym = v;
> +               size_t len;
>
>                 if (sym->owner == NULL)
>                         continue;
>
> -               strcpy(alias + baselen, sym->name);
> +               len = strlen(sym->name);
> +               scratchbuf_alloc(&salias, baselen + len + 1);

err... the whole point of scratchbuf was to be able to increase the
buffer size and check for errors. Here I forgot to check them.


Lucas De Marchi

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 3/3] depmod: fix string overflow
  2016-08-13 20:31   ` Lucas De Marchi
@ 2016-08-15 13:28     ` Lucas De Marchi
  0 siblings, 0 replies; 5+ messages in thread
From: Lucas De Marchi @ 2016-08-15 13:28 UTC (permalink / raw)
  To: linux-modules; +Cc: Lucas De Marchi

On Sat, Aug 13, 2016 at 5:31 PM, Lucas De Marchi
<lucas.de.marchi@gmail.com> wrote:
> On Wed, Aug 10, 2016 at 3:37 PM, Lucas De Marchi
> <lucas.de.marchi@gmail.com> wrote:
>> From: Lucas De Marchi <lucas.demarchi@intel.com>
>>
>> Use scratchbuf to fix issue with strcpy that may overflow the buffer we
>> declared in the stack.
>> ---
>>  tools/depmod.c | 10 +++++++++-
>>  1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/depmod.c b/tools/depmod.c
>> index a2e07c1..be9e001 100644
>> --- a/tools/depmod.c
>> +++ b/tools/depmod.c
>> @@ -35,6 +35,7 @@
>>  #include <shared/hash.h>
>>  #include <shared/macro.h>
>>  #include <shared/util.h>
>> +#include <shared/scratchbuf.h>
>>
>>  #include <libkmod/libkmod.h>
>>
>> @@ -1920,6 +1921,7 @@ static int output_symbols_bin(struct depmod *depmod, FILE *out)
>>  {
>>         struct index_node *idx;
>>         char alias[1024];
>> +       struct scratchbuf salias;
>>         size_t baselen = sizeof("symbol:") - 1;
>>         struct hash_iter iter;
>>         const void *v;
>> @@ -1932,16 +1934,21 @@ static int output_symbols_bin(struct depmod *depmod, FILE *out)
>>                 return -ENOMEM;
>>
>>         memcpy(alias, "symbol:", baselen);
>> +       scratchbuf_init(&salias, alias, sizeof(alias));
>> +
>>         hash_iter_init(depmod->symbols, &iter);
>>
>>         while (hash_iter_next(&iter, NULL, &v)) {
>>                 int duplicate;
>>                 const struct symbol *sym = v;
>> +               size_t len;
>>
>>                 if (sym->owner == NULL)
>>                         continue;
>>
>> -               strcpy(alias + baselen, sym->name);
>> +               len = strlen(sym->name);
>> +               scratchbuf_alloc(&salias, baselen + len + 1);
>
> err... the whole point of scratchbuf was to be able to increase the
> buffer size and check for errors. Here I forgot to check them.

I fixed this and pushed.


Lucas De Marchi

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-10 18:37 [PATCH 1/3] testsuite: include stdio.h Lucas De Marchi
2016-08-10 18:37 ` [PATCH 2/3] Add scratchbuf implementation Lucas De Marchi
2016-08-10 18:37 ` [PATCH 3/3] depmod: fix string overflow Lucas De Marchi
2016-08-13 20:31   ` Lucas De Marchi
2016-08-15 13:28     ` Lucas De Marchi

Linux-Modules Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-modules/0 linux-modules/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-modules linux-modules/ https://lore.kernel.org/linux-modules \
		linux-modules@vger.kernel.org linux-modules@archiver.kernel.org
	public-inbox-index linux-modules


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-modules


AGPL code for this site: git clone https://public-inbox.org/ public-inbox