[PATCH] Enforce strict overflow checking
Andreas Schneider
asn at samba.org
Wed Mar 21 11:21:16 UTC 2018
On Tuesday, 20 March 2018 22:38:44 CET Jeremy Allison wrote:
> On Tue, Mar 20, 2018 at 01:18:39PM -0700, Jeremy Allison via samba-technical
wrote:
> > On Thu, Mar 15, 2018 at 10:16:11AM +0100, Andreas Schneider wrote:
> > > This is a ping :-)
> > >
> > >
> > > Thanks for the review!
> >
> > OK, the only one I'm pushing back on is this:
> Having said that - [PATCH 32/39] lib:param: Fix P_LIST case in
> set_variable_helper() causes:
Here are the latest fixes addressing the remaining issues.
:-)
Please check. Thanks!
--
Andreas Schneider GPG-ID: CC014E3D
Samba Team asn at samba.org
www.samba.org
-------------- next part --------------
>From a1df5b7472c017e855e161b843c12c56efbfb947 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Thu, 7 Dec 2017 18:01:45 +0100
Subject: [PATCH 1/6] s3:printing: Fix size check in get_file_version()
This fixes compilation with -Wstrict-overflow=2
Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Andreas Schneider <asn at samba.org>
---
source3/printing/nt_printing.c | 24 ++++++++++++++++++------
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/source3/printing/nt_printing.c b/source3/printing/nt_printing.c
index 2e500f18c7d..241af37743e 100644
--- a/source3/printing/nt_printing.c
+++ b/source3/printing/nt_printing.c
@@ -485,19 +485,31 @@ static int get_file_version(files_struct *fsp, char *fname,uint32_t *major, uint
/* Potential match data crosses buf boundry, move it to beginning
* of buf, and fill the buf with as much as it will hold. */
if (i>byte_count-VS_VERSION_INFO_SIZE) {
- int bc;
+ ssize_t amount_read;
+ ssize_t amount_unused = byte_count-i;
- memcpy(buf, &buf[i], byte_count-i);
- if ((bc = vfs_read_data(fsp, &buf[byte_count-i], VS_NE_BUF_SIZE-
- (byte_count-i))) < 0) {
+ memmove(buf, &buf[i], amount_unused);
+ amount_read = vfs_read_data(fsp,
+ &buf[amount_unused],
+ VS_NE_BUF_SIZE- amount_unused);
+ if (amount_read < 0) {
DEBUG(0,("get_file_version: NE file [%s] Read error, errno=%d\n",
fname, errno));
goto error_exit;
}
- byte_count = bc + (byte_count - i);
- if (byte_count<VS_VERSION_INFO_SIZE) break;
+ if (amount_read + amount_unused <
+ amount_read) {
+ /* Check for integer wrap. */
+ break;
+ }
+
+ byte_count = amount_read +
+ amount_unused;
+ if (byte_count < VS_VERSION_INFO_SIZE) {
+ break;
+ }
i = 0;
}
--
2.16.2
>From 921fad52898deeb0edb4c275e0ff4aac3a0a792d Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Wed, 21 Mar 2018 11:19:44 +0100
Subject: [PATCH 2/6] s3:lib: Fix size types in ms_fnmatch()
This fixes compilation with -Wstrict-overflow=2
Signed-off-by: Andreas Schneider <asn at samba.org>
---
source3/lib/ms_fnmatch.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/source3/lib/ms_fnmatch.c b/source3/lib/ms_fnmatch.c
index 9763afefe76..a69407b5267 100644
--- a/source3/lib/ms_fnmatch.c
+++ b/source3/lib/ms_fnmatch.c
@@ -150,7 +150,8 @@ int ms_fnmatch(const char *pattern, const char *string, bool translate_pattern,
{
smb_ucs2_t *p = NULL;
smb_ucs2_t *s = NULL;
- int ret, count, i;
+ int ret;
+ size_t count, i;
struct max_n *max_n = NULL;
struct max_n *max_n_free = NULL;
struct max_n one_max_n;
--
2.16.2
>From 5dd569b7c6682c3797dc2d3b9234e5e104177621 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Wed, 21 Mar 2018 11:24:45 +0100
Subject: [PATCH 3/6] s3:lib: Fix size types in tldap_find_first_star()
This fixes compilation with -Wstrict-overflow=2
Signed-off-by: Andreas Schneider <asn at samba.org>
---
source3/lib/tldap.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/source3/lib/tldap.c b/source3/lib/tldap.c
index 205a9cf2b06..bfb24ee8661 100644
--- a/source3/lib/tldap.c
+++ b/source3/lib/tldap.c
@@ -1262,7 +1262,8 @@ static bool tldap_find_first_star(const char *val, const char **star)
static bool tldap_unescape_inplace(char *value, size_t *val_len)
{
- int c, i, p;
+ int c;
+ size_t i, p;
for (i = 0,p = 0; i < *val_len; i++) {
--
2.16.2
>From e67e9fa418ba12237f20a724580044352e8581a8 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Wed, 21 Mar 2018 11:26:55 +0100
Subject: [PATCH 4/6] lib:param: Fix the size type in
lp_do_parameter_parametric()
This fixes compilation with -Wstrict-overflow=2
Signed-off-by: Andreas Schneider <asn at samba.org>
---
lib/param/loadparm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/param/loadparm.c b/lib/param/loadparm.c
index b46700dfb54..0c1b28babbc 100644
--- a/lib/param/loadparm.c
+++ b/lib/param/loadparm.c
@@ -1598,7 +1598,7 @@ static bool lp_do_parameter_parametric(struct loadparm_context *lp_ctx,
static bool set_variable_helper(TALLOC_CTX *mem_ctx, int parmnum, void *parm_ptr,
const char *pszParmName, const char *pszParmValue)
{
- int i;
+ size_t i;
/* switch on the type of variable it is */
switch (parm_table[parmnum].type)
--
2.16.2
>From 9e581a3a8bac311bbbb927ec63be3ec7ab06148f Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Wed, 21 Mar 2018 11:55:45 +0100
Subject: [PATCH 5/6] talloc: Fix size type and checks in _vasprintf_tc
This fixes compilation with -Wstrict-overflow=2
Signed-off-by: Andreas Schneider <asn at samba.org>
---
lib/talloc/talloc.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/lib/talloc/talloc.c b/lib/talloc/talloc.c
index cd159ef89c2..430ebc70f54 100644
--- a/lib/talloc/talloc.c
+++ b/lib/talloc/talloc.c
@@ -2554,7 +2554,8 @@ static struct talloc_chunk *_vasprintf_tc(const void *t,
const char *fmt,
va_list ap)
{
- int len;
+ int vlen;
+ size_t len;
char *ret;
va_list ap2;
struct talloc_chunk *tc;
@@ -2562,9 +2563,13 @@ static struct talloc_chunk *_vasprintf_tc(const void *t,
/* this call looks strange, but it makes it work on older solaris boxes */
va_copy(ap2, ap);
- len = vsnprintf(buf, sizeof(buf), fmt, ap2);
+ vlen = vsnprintf(buf, sizeof(buf), fmt, ap2);
va_end(ap2);
- if (unlikely(len < 0)) {
+ if (unlikely(vlen < 0)) {
+ return NULL;
+ }
+ len = vlen;
+ if (unlikely(len + 1 < len)) {
return NULL;
}
--
2.16.2
>From d41040131077bd611b9ac1ba7f8eec60fcdb729a Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Thu, 7 Dec 2017 15:27:44 +0100
Subject: [PATCH 6/6] wafsamba: Add '-Werror=strict-overflow
-Wstrict-overflow=2' to the developer build
We could move it to 3, but shouldn't go higher. If you set it to 4 and 5
youl will probably also get a lot of false positives.
Signed-off-by: Andreas Schneider <asn at samba.org>
---
buildtools/wafsamba/samba_autoconf.py | 2 ++
1 file changed, 2 insertions(+)
diff --git a/buildtools/wafsamba/samba_autoconf.py b/buildtools/wafsamba/samba_autoconf.py
index 35f4f36f61c..bdd7c8bd195 100644
--- a/buildtools/wafsamba/samba_autoconf.py
+++ b/buildtools/wafsamba/samba_autoconf.py
@@ -713,6 +713,8 @@ def SAMBA_CONFIG_H(conf, path=None):
testflags=True)
conf.ADD_CFLAGS('-Wimplicit-fallthrough',
testflags=True)
+ conf.ADD_CFLAGS('-Werror=strict-overflow -Wstrict-overflow=2',
+ testflags=True)
conf.ADD_CFLAGS('-Wformat=2 -Wno-format-y2k', testflags=True)
conf.ADD_CFLAGS('-Wno-format-zero-length', testflags=True)
--
2.16.2
More information about the samba-technical
mailing list