On Mon, Aug 9, 2021 at 6:54 AM Christian Göttsche <cgzones@xxxxxxxxxxxxxx> wrote: > > The standard function `strerror(3)` is not thread safe. This does not > only affect the concurrent usage of libselinux itself but also with > other `strerror(3)` linked libraries. > Use the thread safe GNU extension format specifier `%m`[1]. > > libselinux already uses the GNU extension format specifier `%ms`. > > [1]: https://www.gnu.org/software/libc/manual/html_node/Other-Output-Conversions.html > > Signed-off-by: Christian Göttsche <cgzones@xxxxxxxxxxxxxx> Acked-by: James Carter <jwcart2@xxxxxxxxx> > --- > libselinux/src/audit2why.c | 11 ++-- > libselinux/src/avc.c | 9 ++-- > libselinux/src/avc_internal.c | 4 +- > libselinux/src/label_backends_android.c | 12 +++-- > libselinux/src/label_file.h | 12 +++-- > libselinux/src/load_policy.c | 20 ++++---- > libselinux/src/matchpathcon.c | 8 +-- > libselinux/src/selinux_restorecon.c | 68 +++++++++++++------------ > 8 files changed, 79 insertions(+), 65 deletions(-) > > diff --git a/libselinux/src/audit2why.c b/libselinux/src/audit2why.c > index 029f874f..ca38e13c 100644 > --- a/libselinux/src/audit2why.c > +++ b/libselinux/src/audit2why.c > @@ -204,8 +204,8 @@ static int __policy_init(const char *init_path) > fp = fopen(path, "re"); > if (!fp) { > snprintf(errormsg, sizeof(errormsg), > - "unable to open %s: %s\n", > - path, strerror(errno)); > + "unable to open %s: %m\n", > + path); > PyErr_SetString( PyExc_ValueError, errormsg); > return 1; > } > @@ -221,9 +221,8 @@ static int __policy_init(const char *init_path) > fp = fopen(curpolicy, "re"); > if (!fp) { > snprintf(errormsg, sizeof(errormsg), > - "unable to open %s: %s\n", > - curpolicy, > - strerror(errno)); > + "unable to open %s: %m\n", > + curpolicy); > PyErr_SetString( PyExc_ValueError, errormsg); > return 1; > } > @@ -242,7 +241,7 @@ static int __policy_init(const char *init_path) > if (sepol_policy_file_create(&pf) || > sepol_policydb_create(&avc->policydb)) { > snprintf(errormsg, sizeof(errormsg), > - "policydb_init failed: %s\n", strerror(errno)); > + "policydb_init failed: %m\n"); > PyErr_SetString( PyExc_RuntimeError, errormsg); > fclose(fp); > return 1; > diff --git a/libselinux/src/avc.c b/libselinux/src/avc.c > index daaedbc6..7493e4b2 100644 > --- a/libselinux/src/avc.c > +++ b/libselinux/src/avc.c > @@ -206,9 +206,8 @@ static int avc_init_internal(const char *prefix, > rc = security_getenforce(); > if (rc < 0) { > avc_log(SELINUX_ERROR, > - "%s: could not determine enforcing mode: %s\n", > - avc_prefix, > - strerror(errno)); > + "%s: could not determine enforcing mode: %m\n", > + avc_prefix); > goto out; > } > avc_enforcing = rc; > @@ -217,8 +216,8 @@ static int avc_init_internal(const char *prefix, > rc = selinux_status_open(0); > if (rc < 0) { > avc_log(SELINUX_ERROR, > - "%s: could not open selinux status page: %d (%s)\n", > - avc_prefix, errno, strerror(errno)); > + "%s: could not open selinux status page: %d (%m)\n", > + avc_prefix, errno); > goto out; > } > avc_running = 1; > diff --git a/libselinux/src/avc_internal.c b/libselinux/src/avc_internal.c > index 53a99a1f..71a1357b 100644 > --- a/libselinux/src/avc_internal.c > +++ b/libselinux/src/avc_internal.c > @@ -308,8 +308,8 @@ int avc_netlink_acquire_fd(void) > rc = avc_netlink_open(0); > if (rc < 0) { > avc_log(SELINUX_ERROR, > - "%s: could not open netlink socket: %d (%s)\n", > - avc_prefix, errno, strerror(errno)); > + "%s: could not open netlink socket: %d (%m)\n", > + avc_prefix, errno); > return rc; > } > } > diff --git a/libselinux/src/label_backends_android.c b/libselinux/src/label_backends_android.c > index cb8aae26..66d4df2d 100644 > --- a/libselinux/src/label_backends_android.c > +++ b/libselinux/src/label_backends_android.c > @@ -94,9 +94,15 @@ static int process_line(struct selabel_handle *rec, > items = read_spec_entries(line_buf, &errbuf, 2, &prop, &context); > if (items < 0) { > items = errno; > - selinux_log(SELINUX_ERROR, > - "%s: line %u error due to: %s\n", path, > - lineno, errbuf ?: strerror(errno)); > + if (errbuf) { > + selinux_log(SELINUX_ERROR, > + "%s: line %u error due to: %s\n", path, > + lineno, errbuf); > + } else { > + selinux_log(SELINUX_ERROR, > + "%s: line %u error due to: %m\n", path, > + lineno); > + } > errno = items; > return -1; > } > diff --git a/libselinux/src/label_file.h b/libselinux/src/label_file.h > index 9f633701..343ffc70 100644 > --- a/libselinux/src/label_file.h > +++ b/libselinux/src/label_file.h > @@ -445,9 +445,15 @@ static inline int process_line(struct selabel_handle *rec, > items = read_spec_entries(line_buf, &errbuf, 3, ®ex, &type, &context); > if (items < 0) { > rc = errno; > - selinux_log(SELINUX_ERROR, > - "%s: line %u error due to: %s\n", path, > - lineno, errbuf ?: strerror(errno)); > + if (errbuf) { > + selinux_log(SELINUX_ERROR, > + "%s: line %u error due to: %s\n", path, > + lineno, errbuf); > + } else { > + selinux_log(SELINUX_ERROR, > + "%s: line %u error due to: %m\n", path, > + lineno); > + } > errno = rc; > return -1; > } > diff --git a/libselinux/src/load_policy.c b/libselinux/src/load_policy.c > index 5857b821..d8c715ed 100644 > --- a/libselinux/src/load_policy.c > +++ b/libselinux/src/load_policy.c > @@ -137,15 +137,15 @@ int selinux_mkload_policy(int preservebools __attribute__((unused))) > } > if (fd < 0) { > fprintf(stderr, > - "SELinux: Could not open policy file <= %s.%d: %s\n", > - selinux_binary_policy_path(), maxvers, strerror(errno)); > + "SELinux: Could not open policy file <= %s.%d: %m\n", > + selinux_binary_policy_path(), maxvers); > goto dlclose; > } > > if (fstat(fd, &sb) < 0) { > fprintf(stderr, > - "SELinux: Could not stat policy file %s: %s\n", > - path, strerror(errno)); > + "SELinux: Could not stat policy file %s: %m\n", > + path); > goto close; > } > > @@ -153,8 +153,8 @@ int selinux_mkload_policy(int preservebools __attribute__((unused))) > data = map = mmap(NULL, size, PROT_READ, MAP_PRIVATE, fd, 0); > if (map == MAP_FAILED) { > fprintf(stderr, > - "SELinux: Could not map policy file %s: %s\n", > - path, strerror(errno)); > + "SELinux: Could not map policy file %s: %m\n", > + path); > goto close; > } > > @@ -193,8 +193,8 @@ int selinux_mkload_policy(int preservebools __attribute__((unused))) > > if (rc) > fprintf(stderr, > - "SELinux: Could not load policy file %s: %s\n", > - path, strerror(errno)); > + "SELinux: Could not load policy file %s: %m\n", > + path); > > unmap: > if (data != map) > @@ -306,7 +306,7 @@ int selinux_init_load_policy(int *enforce) > *enforce = 0; > } else { > /* Only emit this error if selinux was not disabled */ > - fprintf(stderr, "Mount failed for selinuxfs on %s: %s\n", SELINUXMNT, strerror(errno)); > + fprintf(stderr, "Mount failed for selinuxfs on %s: %m\n", SELINUXMNT); > } > > if (rc == 0) > @@ -352,7 +352,7 @@ int selinux_init_load_policy(int *enforce) > if (orig_enforce != *enforce) { > rc = security_setenforce(*enforce); > if (rc < 0) { > - fprintf(stderr, "SELinux: Unable to switch to %s mode: %s\n", (*enforce ? "enforcing" : "permissive"), strerror(errno)); > + fprintf(stderr, "SELinux: Unable to switch to %s mode: %m\n", (*enforce ? "enforcing" : "permissive")); > if (*enforce) > goto noload; > } > diff --git a/libselinux/src/matchpathcon.c b/libselinux/src/matchpathcon.c > index 075a3fb3..1e7f8890 100644 > --- a/libselinux/src/matchpathcon.c > +++ b/libselinux/src/matchpathcon.c > @@ -393,8 +393,8 @@ int realpath_not_final(const char *name, char *resolved_path) > > tmp_path = strdup(name); > if (!tmp_path) { > - myprintf("symlink_realpath(%s) strdup() failed: %s\n", > - name, strerror(errno)); > + myprintf("symlink_realpath(%s) strdup() failed: %m\n", > + name); > rc = -1; > goto out; > } > @@ -414,8 +414,8 @@ int realpath_not_final(const char *name, char *resolved_path) > } > > if (!p) { > - myprintf("symlink_realpath(%s) realpath() failed: %s\n", > - name, strerror(errno)); > + myprintf("symlink_realpath(%s) realpath() failed: %m\n", > + name); > rc = -1; > goto out; > } > diff --git a/libselinux/src/selinux_restorecon.c b/libselinux/src/selinux_restorecon.c > index 999aa924..04d95650 100644 > --- a/libselinux/src/selinux_restorecon.c > +++ b/libselinux/src/selinux_restorecon.c > @@ -333,8 +333,7 @@ static int add_xattr_entry(const char *directory, bool delete_nonmatch, > rc = removexattr(directory, RESTORECON_PARTIAL_MATCH_DIGEST); > if (rc) { > selinux_log(SELINUX_ERROR, > - "Error: %s removing xattr \"%s\" from: %s\n", > - strerror(errno), > + "Error: %m removing xattr \"%s\" from: %s\n", > RESTORECON_PARTIAL_MATCH_DIGEST, directory); > digest_result = ERROR; > } > @@ -734,8 +733,8 @@ out1: > return rc; > err: > selinux_log(SELINUX_ERROR, > - "Could not set context for %s: %s\n", > - pathname, strerror(errno)); > + "Could not set context for %s: %m\n", > + pathname); > rc = -1; > goto out1; > } > @@ -857,6 +856,7 @@ int selinux_restorecon(const char *pathname_orig, > dev_t dev_num = 0; > struct dir_hash_node *current = NULL; > struct dir_hash_node *head = NULL; > + int errno_tmp; > > if (flags.verbose && flags.progress) > flags.verbose = false; > @@ -929,8 +929,8 @@ int selinux_restorecon(const char *pathname_orig, > return 0; > } else { > selinux_log(SELINUX_ERROR, > - "lstat(%s) failed: %s\n", > - pathname, strerror(errno)); > + "lstat(%s) failed: %m\n", > + pathname); > error = -1; > goto cleanup; > } > @@ -954,8 +954,8 @@ int selinux_restorecon(const char *pathname_orig, > memset(&sfsb, 0, sizeof sfsb); > if (!S_ISLNK(sb.st_mode) && statfs(pathname, &sfsb) < 0) { > selinux_log(SELINUX_ERROR, > - "statfs(%s) failed: %s\n", > - pathname, strerror(errno)); > + "statfs(%s) failed: %m\n", > + pathname); > error = -1; > goto cleanup; > } > @@ -1006,24 +1006,30 @@ int selinux_restorecon(const char *pathname_orig, > case FTS_DP: > continue; > case FTS_DNR: > + errno_tmp = errno; > + errno = ftsent->fts_errno; > selinux_log(SELINUX_ERROR, > - "Could not read %s: %s.\n", > - ftsent->fts_path, > - strerror(ftsent->fts_errno)); > + "Could not read %s: %m.\n", > + ftsent->fts_path); > + errno = errno_tmp; > fts_set(fts, ftsent, FTS_SKIP); > continue; > case FTS_NS: > + errno_tmp = errno; > + errno = ftsent->fts_errno; > selinux_log(SELINUX_ERROR, > - "Could not stat %s: %s.\n", > - ftsent->fts_path, > - strerror(ftsent->fts_errno)); > + "Could not stat %s: %m.\n", > + ftsent->fts_path); > + errno = errno_tmp; > fts_set(fts, ftsent, FTS_SKIP); > continue; > case FTS_ERR: > + errno_tmp = errno; > + errno = ftsent->fts_errno; > selinux_log(SELINUX_ERROR, > - "Error on %s: %s.\n", > - ftsent->fts_path, > - strerror(ftsent->fts_errno)); > + "Error on %s: %m.\n", > + ftsent->fts_path); > + errno = errno_tmp; > fts_set(fts, ftsent, FTS_SKIP); > continue; > case FTS_D: > @@ -1087,9 +1093,8 @@ int selinux_restorecon(const char *pathname_orig, > current->digest, > SHA1_HASH_SIZE, 0) < 0) { > selinux_log(SELINUX_ERROR, > - "setxattr failed: %s: %s\n", > - current->path, > - strerror(errno)); > + "setxattr failed: %s: %m\n", > + current->path); > } > current = current->next; > } > @@ -1131,16 +1136,16 @@ oom: > realpatherr: > sverrno = errno; > selinux_log(SELINUX_ERROR, > - "SELinux: Could not get canonical path for %s restorecon: %s.\n", > - pathname_orig, strerror(errno)); > + "SELinux: Could not get canonical path for %s restorecon: %m.\n", > + pathname_orig); > errno = sverrno; > error = -1; > goto cleanup; > > fts_err: > selinux_log(SELINUX_ERROR, > - "fts error while labeling %s: %s\n", > - paths[0], strerror(errno)); > + "fts error while labeling %s: %m\n", > + paths[0]); > error = -1; > goto cleanup; > } > @@ -1181,8 +1186,7 @@ struct selabel_handle *selinux_restorecon_default_handle(void) > > if (!sehandle) { > selinux_log(SELINUX_ERROR, > - "Error obtaining file context handle: %s\n", > - strerror(errno)); > + "Error obtaining file context handle: %m\n"); > return NULL; > } > > @@ -1202,8 +1206,8 @@ void selinux_restorecon_set_exclude_list(const char **exclude_list) > for (i = 0; exclude_list[i]; i++) { > if (lstat(exclude_list[i], &sb) < 0 && errno != EACCES) { > selinux_log(SELINUX_ERROR, > - "lstat error on exclude path \"%s\", %s - ignoring.\n", > - exclude_list[i], strerror(errno)); > + "lstat error on exclude path \"%s\", %m - ignoring.\n", > + exclude_list[i]); > break; > } > if (add_exclude(exclude_list[i], CALLER_EXCLUDED) && > @@ -1269,8 +1273,8 @@ int selinux_restorecon_xattr(const char *pathname, unsigned int xattr_flags, > return 0; > > selinux_log(SELINUX_ERROR, > - "lstat(%s) failed: %s\n", > - pathname, strerror(errno)); > + "lstat(%s) failed: %m\n", > + pathname); > return -1; > } > > @@ -1300,8 +1304,8 @@ int selinux_restorecon_xattr(const char *pathname, unsigned int xattr_flags, > fts = fts_open(paths, fts_flags, NULL); > if (!fts) { > selinux_log(SELINUX_ERROR, > - "fts error on %s: %s\n", > - paths[0], strerror(errno)); > + "fts error on %s: %m\n", > + paths[0]); > return -1; > } > > -- > 2.32.0 >
