URL: https://github.com/SSSD/sssd/pull/508 Author: lslebodn Title: #508: Fix/suppress few gcc8 warnings Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/508/head:pr508 git checkout pr508
From 2da148cbf807916af0282bb552dff6cd171608fd Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik <lsleb...@redhat.com> Date: Mon, 5 Feb 2018 15:34:01 +0100 Subject: [PATCH 1/7] AD: Suppress warning Wincompatible-pointer-types with sasl callbacks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SASL use different prototype for callbacks based on id. However struct sasl_callback_t contains generic callback int (*)(void) which is not compatible with these callbacks and gcc8 warns about it. src/providers/ad/ad_init.c:116:23: warning: cast between incompatible function types from ‘int (*)(void *, const char *, const char *, const char **, unsigned int *)’ to ‘int (*)(void)’ [-Wcast-function-type] { SASL_CB_GETOPT, (sss_sasl_gen_cb_fn)ad_sasl_getopt, NULL }, ^ src/providers/ad/ad_init.c:117:20: warning: cast between incompatible function types from ‘int (*)(void *, int, const char *)’ to ‘int (*)(void)’ [-Wcast-function-type] { SASL_CB_LOG, (sss_sasl_gen_cb_fn)ad_sasl_log, NULL }, ^ --- src/providers/ad/ad_init.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/providers/ad/ad_init.c b/src/providers/ad/ad_init.c index e6f3316e2..8c485a7c2 100644 --- a/src/providers/ad/ad_init.c +++ b/src/providers/ad/ad_init.c @@ -113,8 +113,8 @@ static int ad_sasl_log(void *context, int level, const char *message) } static const sasl_callback_t ad_sasl_callbacks[] = { - { SASL_CB_GETOPT, (sss_sasl_gen_cb_fn)ad_sasl_getopt, NULL }, - { SASL_CB_LOG, (sss_sasl_gen_cb_fn)ad_sasl_log, NULL }, + { SASL_CB_GETOPT, (sss_sasl_gen_cb_fn)(void *)ad_sasl_getopt, NULL }, + { SASL_CB_LOG, (sss_sasl_gen_cb_fn)(void *)ad_sasl_log, NULL }, { SASL_CB_LIST_END, NULL, NULL } }; From d776e3af8c0e9d36bfbf100d61e76217454031de Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik <lsleb...@redhat.com> Date: Mon, 5 Feb 2018 15:10:14 +0100 Subject: [PATCH 2/7] pysss: Drop unused parameter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit struct PyMethodDef has PyCFunction in definition as ml_meth and for py_sss_encrypt we needn't use PyCFunctionWithKeywords src/python/pysss.c:1082:40: warning: cast between incompatible function types from ‘PyObject * (*)(PySssPasswordObject *, PyObject *, PyObject *)’ {aka ‘struct _object * (*)(struct <anonymous> *, struct _object *, struct _object *)’} to ‘PyObject * (*)(PyObject *, PyObject *)’ {aka ‘struct _object * (*)(struct _object *, struct _object *)’} [-Wcast-function-type] { sss_py_const_p(char, "encrypt"), (PyCFunction) py_sss_encrypt, ^ --- src/python/pysss.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/python/pysss.c b/src/python/pysss.c index dc1c1ff1f..45e20c2c3 100644 --- a/src/python/pysss.c +++ b/src/python/pysss.c @@ -963,8 +963,7 @@ PyDoc_STRVAR(py_sss_encrypt__doc__, ":param method: The obfuscation method\n\n"); static PyObject *py_sss_encrypt(PySssPasswordObject *self, - PyObject *args, - PyObject *kwds) + PyObject *args) { char *password = NULL; int plen; /* may contain NULL bytes */ From 54e97a4b37e619f6a838354e5e682048edfcb656 Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik <lsleb...@redhat.com> Date: Mon, 5 Feb 2018 15:07:13 +0100 Subject: [PATCH 3/7] pysss: Suppress warning Wincompatible-pointer-types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit struct PyMethodDef has PyCFunction in definition as ml_meth but we are using PyCFunctionWithKeywords src/python/pysss.c:910:40: warning: initialization of ‘PyObject * (*)(PyObject *, PyObject *)’ {aka ‘struct _object * (*)(struct _object *, struct _object *)’} from incompatible pointer type ‘PyObject * (*)(PyObject *, PyObject *, PyObject *)’ {aka ‘struct _object * (*)(struct _object *, struct _object *, struct _object *)’} [-Wincompatible-pointer-types] { sss_py_const_p(char, "useradd"), (PyCFunction) py_sss_useradd, ^ src/python/pysss.c:910:40: note: (near initialization for ‘sss_local_methods[0].ml_meth’) --- src/python/pysss.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/python/pysss.c b/src/python/pysss.c index 45e20c2c3..16c086a8b 100644 --- a/src/python/pysss.c +++ b/src/python/pysss.c @@ -902,22 +902,22 @@ static PyObject *PySssLocalObject_new(PyTypeObject *type, * sss.local object methods */ static PyMethodDef sss_local_methods[] = { - { sss_py_const_p(char, "useradd"), (PyCFunction) py_sss_useradd, + { sss_py_const_p(char, "useradd"), (PyCFunction)(void *) py_sss_useradd, METH_KEYWORDS, py_sss_useradd__doc__ }, - { sss_py_const_p(char, "userdel"), (PyCFunction) py_sss_userdel, + { sss_py_const_p(char, "userdel"), (PyCFunction)(void *) py_sss_userdel, METH_KEYWORDS, py_sss_userdel__doc__ }, - { sss_py_const_p(char, "usermod"), (PyCFunction) py_sss_usermod, + { sss_py_const_p(char, "usermod"), (PyCFunction)(void *) py_sss_usermod, METH_KEYWORDS, py_sss_usermod__doc__ }, - { sss_py_const_p(char, "groupadd"), (PyCFunction) py_sss_groupadd, + { sss_py_const_p(char, "groupadd"), (PyCFunction)(void *) py_sss_groupadd, METH_KEYWORDS, py_sss_groupadd__doc__ }, - { sss_py_const_p(char, "groupdel"), (PyCFunction) py_sss_groupdel, + { sss_py_const_p(char, "groupdel"), (PyCFunction)(void *) py_sss_groupdel, METH_KEYWORDS, py_sss_groupdel__doc__ }, - { sss_py_const_p(char, "groupmod"), (PyCFunction) py_sss_groupmod, + { sss_py_const_p(char, "groupmod"), (PyCFunction)(void *) py_sss_groupmod, METH_KEYWORDS, py_sss_groupmod__doc__ }, {NULL, NULL, 0, NULL} /* Sentinel */ From 1403fc21e0647689d80c194939e5a80ea0cc8579 Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik <lsleb...@redhat.com> Date: Sat, 3 Feb 2018 19:04:58 +0000 Subject: [PATCH 4/7] CRYPTO: Suppress warning Wstringop-truncation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit src/util/crypto/nss/nss_sha512crypt.c: In function ‘sha512_crypt_r’: src/util/crypto/nss/nss_sha512crypt.c:270:10: warning: ‘stpncpy’ output truncated copying 3 bytes from a string of length 3 [-Wstringop-truncation] cp = stpncpy(buffer, sha512_salt_prefix, SALT_PREF_SIZE); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ man gcc says: As another example, the following call to "strncpy" results in copying to "d" just the characters preceding the terminating NUL, without appending the NUL to the end. Assuming the result of "strncpy" is necessarily a NUL-terminated string is a common mistake, and so the call is diagnosed. To avoid the warning when the result is not expected to be NUL-terminated, call "memcpy" instead. void copy (char *d, const char *s) { strncpy (d, s, strlen (s)); } --- src/util/crypto/libcrypto/crypto_sha512crypt.c | 2 +- src/util/crypto/nss/nss_sha512crypt.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/crypto/libcrypto/crypto_sha512crypt.c b/src/util/crypto/libcrypto/crypto_sha512crypt.c index b074eee55..5861f34b9 100644 --- a/src/util/crypto/libcrypto/crypto_sha512crypt.c +++ b/src/util/crypto/libcrypto/crypto_sha512crypt.c @@ -277,7 +277,7 @@ static int sha512_crypt_r(const char *key, goto done; } - cp = stpncpy(buffer, sha512_salt_prefix, SALT_PREF_SIZE); + cp = memcpy(buffer, sha512_salt_prefix, SALT_PREF_SIZE); buflen -= SALT_PREF_SIZE; if (rounds_custom) { diff --git a/src/util/crypto/nss/nss_sha512crypt.c b/src/util/crypto/nss/nss_sha512crypt.c index 2f1624e63..709cf5196 100644 --- a/src/util/crypto/nss/nss_sha512crypt.c +++ b/src/util/crypto/nss/nss_sha512crypt.c @@ -267,7 +267,7 @@ static int sha512_crypt_r(const char *key, goto done; } - cp = stpncpy(buffer, sha512_salt_prefix, SALT_PREF_SIZE); + cp = memcpy(buffer, sha512_salt_prefix, SALT_PREF_SIZE); buflen -= SALT_PREF_SIZE; if (rounds_custom) { From e80a149faba3ad45d787a9baf40ac356e928957c Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik <lsleb...@redhat.com> Date: Mon, 5 Feb 2018 16:36:24 +0100 Subject: [PATCH 5/7] INOTIFY: Fix warning Wstringop-truncation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It could not cause any security bug because it is used only with short names /etc/passwd, /etc/group, /etc/resolv.conf. And only root could set long names via env variables SSS_FILES_PASSWD, SSS_FILES_GROUP CC src/util/libsss_files_la-inotify.lo src/util/inotify.c: In function ‘copy_filenames’: src/util/inotify.c:390:5: warning: ‘strncpy’ specified bound 4096 equals destination size [-Wstringop-truncation] strncpy(fcopy, filename, sizeof(fcopy)); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ../sssd/src/util/inotify.c:403:5: warning: ‘strncpy’ specified bound 4096 equals destination size [-Wstringop-truncation] strncpy(fcopy, filename, sizeof(fcopy)); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ man gcc says: In the following example, the call to "strncpy" specifies the size of the destination buffer as the bound. If the length of the source string is equal to or greater than this size the result of the copy will not be NUL-terminated. Therefore, the call is also diagnosed. To avoid the warning, specify "sizeof buf - 1" as the bound and set the last element of the buffer to "NUL". void copy (const char *s) { char buf[80]; strncpy (buf, s, sizeof buf); ... } --- src/util/inotify.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/util/inotify.c b/src/util/inotify.c index 73dc8a695..2e2dc1a6e 100644 --- a/src/util/inotify.c +++ b/src/util/inotify.c @@ -385,10 +385,10 @@ static errno_t copy_filenames(struct snotify_ctx *snctx, const char *filename) { char *p; - char fcopy[PATH_MAX]; + char fcopy[PATH_MAX + 1]; - strncpy(fcopy, filename, sizeof(fcopy)); - fcopy[PATH_MAX-1] = '\0'; + strncpy(fcopy, filename, sizeof(fcopy) - 1); + fcopy[PATH_MAX] = '\0'; p = dirname(fcopy); if (p == NULL) { @@ -400,8 +400,8 @@ static errno_t copy_filenames(struct snotify_ctx *snctx, return ENOMEM; } - strncpy(fcopy, filename, sizeof(fcopy)); - fcopy[PATH_MAX-1] = '\0'; + strncpy(fcopy, filename, sizeof(fcopy) - 1); + fcopy[PATH_MAX] = '\0'; p = basename(fcopy); if (p == NULL) { From 4b744d16f7e8695c477eed58bf094be0523cf02e Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik <lsleb...@redhat.com> Date: Mon, 5 Feb 2018 16:16:38 +0100 Subject: [PATCH 6/7] SIFP: Suppress warning Wstringop-truncation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It is a false positive because we allocate 0 initialized function with size "str_len + 1". Result will be always NUL terminated. src/lib/sifp/sss_sifp_utils.c: In function ‘sss_sifp_strdup’: src/lib/sifp/sss_sifp_utils.c:62:5: warning: ‘strncpy’ output truncated before terminating nul copying as many bytes from a string as its length [-Wstringop-truncation] strncpy(result, str, str_len); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ src/lib/sifp/sss_sifp_utils.c:56:15: note: length computed here str_len = strlen(str); ^~~~~~~~~~~ --- src/lib/sifp/sss_sifp_utils.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/sifp/sss_sifp_utils.c b/src/lib/sifp/sss_sifp_utils.c index dcac71f50..36cbb83a9 100644 --- a/src/lib/sifp/sss_sifp_utils.c +++ b/src/lib/sifp/sss_sifp_utils.c @@ -59,7 +59,7 @@ char * sss_sifp_strdup(sss_sifp_ctx *ctx, const char *str) return NULL; } - strncpy(result, str, str_len); + memcpy(result, str, str_len); return result; } From 256ed5622071985c12e33ecd73a555caf0a6993c Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik <lsleb...@redhat.com> Date: Mon, 5 Feb 2018 16:46:16 +0100 Subject: [PATCH 7/7] CLIENT: Fix warning Wstringop-overflow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It could not cause any problem because all strings were compile time constants (defines). But it's better to check that we have enough data for storing NUL terminated string in nssaddr.sun_path src/sss_client/common.c: In function ‘sss_cli_check_socket’: src/sss_client/common.c:544:5: warning: ‘strncpy’ specified bound depends on the length of the source argument [-Wstringop-overflow=] strncpy(nssaddr.sun_path, socket_name, ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ strlen(socket_name) + 1); ~~~~~~~~~~~~~~~~~~~~~~~~ src/sss_client/common.c:545:13: note: length computed here strlen(socket_name) + 1); ^~~~~~~~~~~~~~~~~~~ --- src/sss_client/common.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/sss_client/common.c b/src/sss_client/common.c index 17fc8cb7d..67a460705 100644 --- a/src/sss_client/common.c +++ b/src/sss_client/common.c @@ -539,10 +539,14 @@ static int sss_cli_open_socket(int *errnop, const char *socket_name, int timeout int ret; int sd; + if (sizeof(nssaddr.sun_path) <= strlen(socket_name) + 1) { + *errnop = EINVAL; + return -1; + } + memset(&nssaddr, 0, sizeof(struct sockaddr_un)); nssaddr.sun_family = AF_UNIX; - strncpy(nssaddr.sun_path, socket_name, - strlen(socket_name) + 1); + strncpy(nssaddr.sun_path, socket_name, sizeof(nssaddr.sun_path)); sd = socket(AF_UNIX, SOCK_STREAM, 0); if (sd == -1) {
_______________________________________________ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org