summaryrefslogtreecommitdiffstats
path: root/xlators/features
diff options
context:
space:
mode:
authorYaniv Kaul <ykaul@redhat.com>2018-08-21 19:23:01 +0300
committerAmar Tumballi <amarts@redhat.com>2018-08-31 06:14:47 +0000
commitdc6e6b71f87f6f89bb0b69816e92779595d716bd (patch)
treee9fbd7f4384a6ccb05a3537b064588ee30f1b6be /xlators/features
parent058d215174b93b3aa14be99073979f45642e519e (diff)
changelog xlator: strncpy()->sprintf(), reduce strlen()'s
xlators/features/changelog/lib/src/gf-changelog-journal-handler.c xlators/features/changelog/lib/src/gf-changelog.c xlators/features/changelog/src/changelog-helpers.c xlators/features/changelog/src/changelog-misc.h strncpy may not be very efficient for short strings copied into a large buffer: If the length of src is less than n, strncpy() writes additional null bytes to dest to ensure that a total of n bytes are written. Instead, use snprintf(). Where possible, ensure there's no truncation of the output. Also: - save the result of strlen() and re-use it when possible. - move from strlen to SLEN (sizeof() ) for const strings. - switch a strncpy to a memcpy. Compile-tested only! Change-Id: Ia7a52bce0b243613ad910192ec163c93d944e077 updates: bz#1193929 Signed-off-by: Yaniv Kaul <ykaul@redhat.com>
Diffstat (limited to 'xlators/features')
-rw-r--r--xlators/features/changelog/lib/src/gf-changelog-journal-handler.c36
-rw-r--r--xlators/features/changelog/lib/src/gf-changelog.c5
-rw-r--r--xlators/features/changelog/src/changelog-helpers.c43
-rw-r--r--xlators/features/changelog/src/changelog-misc.h8
4 files changed, 55 insertions, 37 deletions
diff --git a/xlators/features/changelog/lib/src/gf-changelog-journal-handler.c b/xlators/features/changelog/lib/src/gf-changelog-journal-handler.c
index 9c1a498f655..bdb410030f6 100644
--- a/xlators/features/changelog/lib/src/gf-changelog-journal-handler.c
+++ b/xlators/features/changelog/lib/src/gf-changelog-journal-handler.c
@@ -456,7 +456,7 @@ gf_changelog_decode (xlator_t *this, gf_changelog_journal_t *jnl,
size_t elen = 0;
char buffer[1024] = {0,};
- CHANGELOG_GET_HEADER_INFO (from_fd, buffer, 1024, encoding,
+ CHANGELOG_GET_HEADER_INFO (from_fd, buffer, sizeof (buffer), encoding,
major_version, minor_version, elen);
if (encoding == -1) /* unknown encoding */
goto out;
@@ -521,8 +521,9 @@ gf_changelog_publish (xlator_t *this,
char to_path[PATH_MAX] = {0,};
struct stat stbuf = {0,};
- (void) snprintf (to_path, PATH_MAX, "%s%s",
- jnl->jnl_current_dir, basename (from_path));
+ if (snprintf (to_path, PATH_MAX, "%s%s", jnl->jnl_current_dir,
+ basename (from_path)) >= PATH_MAX)
+ return -1;
/* handle zerob file that won't exist in current */
ret = sys_stat (to_path, &stbuf);
@@ -532,8 +533,9 @@ gf_changelog_publish (xlator_t *this,
goto out;
}
- (void) snprintf (dest, PATH_MAX, "%s%s",
- jnl->jnl_processing_dir, basename (from_path));
+ if (snprintf (dest, PATH_MAX, "%s%s", jnl->jnl_processing_dir,
+ basename (from_path)) >= PATH_MAX)
+ return -1;
ret = sys_rename (to_path, dest);
if (ret) {
@@ -561,6 +563,13 @@ gf_changelog_consume (xlator_t *this,
char dest[PATH_MAX] = {0,};
char to_path[PATH_MAX] = {0,};
+ if (snprintf (to_path, PATH_MAX, "%s%s", jnl->jnl_current_dir,
+ basename (from_path)) >= PATH_MAX)
+ goto out;
+ if (snprintf (dest, PATH_MAX, "%s%s", jnl->jnl_processing_dir,
+ basename (from_path)) >= PATH_MAX)
+ goto out;
+
ret = sys_stat (from_path, &stbuf);
if (ret || !S_ISREG(stbuf.st_mode)) {
ret = -1;
@@ -582,11 +591,6 @@ gf_changelog_consume (xlator_t *this,
goto out;
}
- (void) snprintf (to_path, PATH_MAX, "%s%s",
- jnl->jnl_current_dir, basename (from_path));
- (void) snprintf (dest, PATH_MAX, "%s%s",
- jnl->jnl_processing_dir, basename (from_path));
-
fd2 = open (to_path, O_CREAT | O_TRUNC | O_RDWR,
S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
if (fd2 < 0) {
@@ -948,8 +952,9 @@ gf_changelog_init_history (xlator_t *this,
goto dealloc_hist;
}
- (void) strncpy (jnl->hist_jnl->jnl_brickpath, brick_path, PATH_MAX-1);
- jnl->hist_jnl->jnl_brickpath[PATH_MAX-1] = 0;
+ if (snprintf (jnl->hist_jnl->jnl_brickpath, PATH_MAX, "%s",
+ brick_path) >= PATH_MAX)
+ goto dealloc_hist;
for (i = 0; i < 256; i++) {
jnl->hist_jnl->rfc3986_space_newline[i] =
@@ -999,6 +1004,10 @@ gf_changelog_journal_init (void *xl, struct gf_brick_spec *brick)
if (!jnl)
goto error_return;
+ if (snprintf (jnl->jnl_brickpath, PATH_MAX, "%s",
+ brick->brick_path) >= PATH_MAX)
+ goto dealloc_private;
+
if (sys_stat (scratch_dir, &buf) && errno == ENOENT) {
ret = mkdir_p (scratch_dir, 0600, _gf_true);
if (ret)
@@ -1017,9 +1026,6 @@ gf_changelog_journal_init (void *xl, struct gf_brick_spec *brick)
goto dealloc_private;
}
- (void) strncpy (jnl->jnl_brickpath, brick->brick_path, PATH_MAX-1);
- jnl->jnl_brickpath[PATH_MAX-1] = 0;
-
/* RFC 3986 {de,en}coding */
for (i = 0; i < 256; i++) {
jnl->rfc3986_space_newline[i] =
diff --git a/xlators/features/changelog/lib/src/gf-changelog.c b/xlators/features/changelog/lib/src/gf-changelog.c
index 6ad6c63740d..8198560e736 100644
--- a/xlators/features/changelog/lib/src/gf-changelog.c
+++ b/xlators/features/changelog/lib/src/gf-changelog.c
@@ -374,8 +374,9 @@ gf_setup_brick_connection (xlator_t *this,
entry->connstate = GF_CHANGELOG_CONN_STATE_PENDING;
entry->notify = brick->filter;
- (void) strncpy (entry->brick, brick->brick_path, PATH_MAX-1);
- entry->brick[PATH_MAX-1] = 0;
+ if (snprintf (entry->brick, PATH_MAX, "%s", brick->brick_path)
+ >= PATH_MAX)
+ goto free_entry;
entry->this = this;
entry->invokerxl = xl;
diff --git a/xlators/features/changelog/src/changelog-helpers.c b/xlators/features/changelog/src/changelog-helpers.c
index bfd2a29bdb1..28c72a8f9d4 100644
--- a/xlators/features/changelog/src/changelog-helpers.c
+++ b/xlators/features/changelog/src/changelog-helpers.c
@@ -290,10 +290,11 @@ htime_update (xlator_t *this,
ret = -1;
goto out;
}
- strncpy (changelog_path, buffer, PATH_MAX);
- len = strlen (changelog_path);
- changelog_path[len] = '\0'; /* redundant */
-
+ len = snprintf(changelog_path, PATH_MAX, "%s", buffer);
+ if (len >= PATH_MAX) {
+ ret = -1;
+ goto out;
+ }
if (changelog_write (priv->htime_fd, (void*) changelog_path, len+1 ) < 0) {
gf_msg (this->name, GF_LOG_ERROR, 0,
CHANGELOG_MSG_HTIME_ERROR,
@@ -302,11 +303,15 @@ htime_update (xlator_t *this,
goto out;
}
- snprintf (x_value, sizeof x_value, "%lu:%d",
- ts, priv->rollover_count);
+ len = snprintf (x_value, sizeof (x_value), "%lu:%d",
+ ts, priv->rollover_count);
+ if (len >= sizeof (x_value)) {
+ ret = -1;
+ goto out;
+ }
if (sys_fsetxattr (priv->htime_fd, HTIME_KEY, x_value,
- strlen (x_value), XATTR_REPLACE)) {
+ len, XATTR_REPLACE)) {
gf_smsg (this->name, GF_LOG_ERROR, errno,
CHANGELOG_MSG_HTIME_ERROR,
"Htime xattr updation failed with XATTR_REPLACE",
@@ -314,7 +319,7 @@ htime_update (xlator_t *this,
NULL);
if (sys_fsetxattr (priv->htime_fd, HTIME_KEY, x_value,
- strlen (x_value), 0)) {
+ len, 0)) {
gf_smsg (this->name, GF_LOG_ERROR, errno,
CHANGELOG_MSG_HTIME_ERROR,
"Htime xattr updation failed",
@@ -366,7 +371,7 @@ cl_is_empty (xlator_t *this, int fd)
goto out;
}
- CHANGELOG_GET_HEADER_INFO (fd, buffer, 1024, encoding,
+ CHANGELOG_GET_HEADER_INFO (fd, buffer, sizeof (buffer), encoding,
major_version, minor_version, elen);
if (elen == stbuf.st_size) {
@@ -390,8 +395,8 @@ out:
int
update_path (xlator_t *this, char *cl_path)
{
- char low_cl[] = "changelog";
- char up_cl[] = "CHANGELOG";
+ const char low_cl[] = "changelog";
+ const char up_cl[] = "CHANGELOG";
char *found = NULL;
int ret = -1;
@@ -403,7 +408,7 @@ update_path (xlator_t *this, char *cl_path)
"Could not find CHANGELOG in changelog path");
goto out;
} else {
- strncpy(found, low_cl, strlen(low_cl));
+ memcpy(found, low_cl, sizeof (low_cl) - 1);
}
ret = 0;
@@ -574,9 +579,11 @@ find_current_htime (int ht_dir_fd, const char *ht_dir_path, char *ht_file_bname)
CHANGELOG_MSG_SCAN_DIR_FAILED,
"scandir failed");
} else if (cnt > 0) {
- strncpy (ht_file_bname, namelist[cnt - 1]->d_name, NAME_MAX);
- ht_file_bname[NAME_MAX - 1] = 0;
-
+ if (snprintf (ht_file_bname, NAME_MAX, "%s",
+ namelist[cnt - 1]->d_name) >= NAME_MAX) {
+ ret = -1;
+ goto out;
+ }
if (sys_fsetxattr (ht_dir_fd, HTIME_CURRENT, ht_file_bname,
strlen (ht_file_bname), 0)) {
gf_msg (this->name, GF_LOG_ERROR, errno,
@@ -2055,7 +2062,11 @@ resolve_pargfid_to_path (xlator_t *this, const uuid_t pgfid,
ret = -1;
goto out;
}
- strncpy (pre_dir_name, result, sizeof(pre_dir_name));
+ if (snprintf (pre_dir_name, len + 1, "%s", result)
+ >= len + 1) {
+ ret = -1;
+ goto out;
+ }
gf_uuid_parse (pgfidstr, tmp_gfid);
gf_uuid_copy (pargfid, tmp_gfid);
diff --git a/xlators/features/changelog/src/changelog-misc.h b/xlators/features/changelog/src/changelog-misc.h
index 93af201879e..e96533f7365 100644
--- a/xlators/features/changelog/src/changelog-misc.h
+++ b/xlators/features/changelog/src/changelog-misc.h
@@ -86,13 +86,13 @@
} while (0)
#define CHANGELOG_FILL_HTIME_DIR(changelog_dir, path) do { \
- strncpy (path, changelog_dir, sizeof (path) - 1); \
- strcat (path, "/htime"); \
+ snprintf (path, sizeof (path), "%s/htime", \
+ changelog_dir); \
} while(0)
#define CHANGELOG_FILL_CSNAP_DIR(changelog_dir, path) do { \
- strncpy (path, changelog_dir, sizeof (path) - 1); \
- strcat (path, "/csnap"); \
+ snprintf (path, sizeof (path), "%s/csnap", \
+ changelog_dir); \
} while(0)
/**
* everything after 'CHANGELOG_TYPE_METADATA_XATTR' are internal types