shaba/lvm2
1
0
Fork 0
mirror of git://sourceware.org/git/lvm2.git synced 2024-12-21 13:34:40 +03:00
Code Releases Activity

lvmlocks: rework dm_strncpy

Browse Source 
Fix cutting away 1 character via incorrect usage of dm_strncpy
introduced in last batch of commits and use sizeof(buffer) to
get proper sizes.

In case of use strcpy_name_len() the replacement was invalid,
so we need to restore this case since sanlock uses buffer without
nul ending, so we would strip one more character from the buffer.

Also start to use dm_strncpy() without (void) for unchecked usage.
This commit is contained in:
Zdenek Kabelac 2024-04-03 21:01:40 +02:00
parent f9fefaaabe
commit 5dec664ccf
3 changed files with 51 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
daemons/lvmlockd/lvmlockd-core.c
View File

@ -2367,13 +2367,13 @@ static struct resource *find_resource_act(struct lockspace *ls,
r->mode = LD_LK_UN; r->mode = LD_LK_UN;
if (r->type == LD_RT_GL) { if (r->type == LD_RT_GL) {
(void)dm_strncpy(r->name, R_NAME_GL, MAX_NAME); dm_strncpy(r->name, R_NAME_GL, sizeof(r->name));
r->use_vb = 1; r->use_vb = 1;
} else if (r->type == LD_RT_VG) { } else if (r->type == LD_RT_VG) {
(void)dm_strncpy(r->name, R_NAME_VG, MAX_NAME); dm_strncpy(r->name, R_NAME_VG, sizeof(r->name));
r->use_vb = 1; r->use_vb = 1;
} else if (r->type == LD_RT_LV) { } else if (r->type == LD_RT_LV) {
(void)dm_strncpy(r->name, act->lv_uuid, MAX_NAME); dm_strncpy(r->name, act->lv_uuid, sizeof(r->name));
r->use_vb = 0; r->use_vb = 0;
} }
@ -3529,7 +3529,7 @@ static void work_test_gl(void)
is_enabled = lm_gl_is_enabled(ls); is_enabled = lm_gl_is_enabled(ls);
if (is_enabled) { if (is_enabled) {
log_debug("S %s worker found gl_is_enabled", ls->name); log_debug("S %s worker found gl_is_enabled", ls->name);
(void)dm_strncpy(gl_lsname_sanlock, ls->name, MAX_NAME); dm_strncpy(gl_lsname_sanlock, ls->name, sizeof(gl_lsname_sanlock));
} }
} }
pthread_mutex_unlock(&ls->mutex); pthread_mutex_unlock(&ls->mutex);
@ -5719,9 +5719,9 @@ static void adopt_locks(void)
act->flags = (LD_AF_ADOPT | LD_AF_PERSISTENT); act->flags = (LD_AF_ADOPT | LD_AF_PERSISTENT);
act->client_id = INTERNAL_CLIENT_ID; act->client_id = INTERNAL_CLIENT_ID;
act->lm_type = ls->lm_type; act->lm_type = ls->lm_type;
(void)dm_strncpy(act->vg_name, ls->vg_name, MAX_NAME); dm_strncpy(act->vg_name, ls->vg_name, sizeof(act->vg_name));
(void)dm_strncpy(act->lv_uuid, r->name, MAX_NAME); dm_strncpy(act->lv_uuid, r->name, sizeof(act->lv_uuid));
(void)dm_strncpy(act->lv_args, r->lv_args, MAX_ARGS); dm_strncpy(act->lv_args, r->lv_args, sizeof(act->lv_args));
log_debug("adopt lock for lv %s %s", act->vg_name, act->lv_uuid); log_debug("adopt lock for lv %s %s", act->vg_name, act->lv_uuid);
@ -5747,7 +5747,7 @@ static void adopt_locks(void)
act->flags = LD_AF_ADOPT; act->flags = LD_AF_ADOPT;
act->client_id = INTERNAL_CLIENT_ID; act->client_id = INTERNAL_CLIENT_ID;
act->lm_type = ls->lm_type; act->lm_type = ls->lm_type;
(void)dm_strncpy(act->vg_name, ls->vg_name, MAX_NAME); dm_strncpy(act->vg_name, ls->vg_name, sizeof(act->vg_name));
log_debug("adopt lock for vg %s", act->vg_name); log_debug("adopt lock for vg %s", act->vg_name);

6
daemons/lvmlockd/lvmlockd-dlm.c
View File

@ -848,8 +848,8 @@ int lm_get_lockspaces_dlm(struct list_head *ls_rejoin)
} }
ls->lm_type = LD_LM_DLM; ls->lm_type = LD_LM_DLM;
(void)dm_strncpy(ls->name, de->d_name, MAX_NAME); dm_strncpy(ls->name, de->d_name, sizeof(ls->name));
(void)dm_strncpy(ls->vg_name, ls->name + strlen(LVM_LS_PREFIX), MAX_NAME); dm_strncpy(ls->vg_name, ls->name + strlen(LVM_LS_PREFIX), sizeof(ls->vg_name));
list_add_tail(&ls->list, ls_rejoin); list_add_tail(&ls->list, ls_rejoin);
} }
@ -885,7 +885,7 @@ int lm_refresh_lv_start_dlm(struct action *act)
int rv; int rv;
/* split /dev/vgname/lvname into vgname and lvname strings */ /* split /dev/vgname/lvname into vgname and lvname strings */
(void)dm_strncpy(path, act->path, PATH_MAX); dm_strncpy(path, act->path, sizeof(path));
/* skip past dev */ /* skip past dev */
if (!(p = strchr(path + 1, '/'))) if (!(p = strchr(path + 1, '/')))

64
daemons/lvmlockd/lvmlockd-sanlock.c
View File

@ -227,6 +227,22 @@ int lm_data_size_sanlock(void)
static uint64_t daemon_test_lv_count; static uint64_t daemon_test_lv_count;
/*
* Copy a null-terminated string "str" into a fixed
* size struct field "buf" which is not null terminated.
* (ATM SANLK_NAME_LEN is only 48 bytes.
* Avoid strncpy() for coverity issues.
*/
static void strcpy_name_len(char *buf, const char *str, size_t len)
{
size_t l;
/* copy at most len sized length of str */
for (l = 0; l < len; ++l)
if (!(buf[l] = str[l]))
break;
}
static int lock_lv_name_from_args(char *vg_args, char *lock_lv_name) static int lock_lv_name_from_args(char *vg_args, char *lock_lv_name)
{ {
return last_string_from_args(vg_args, lock_lv_name); return last_string_from_args(vg_args, lock_lv_name);
@ -531,7 +547,7 @@ int lm_init_vg_sanlock(char *ls_name, char *vg_name, uint32_t flags, char *vg_ar
VG_LOCK_ARGS_MAJOR, VG_LOCK_ARGS_MINOR, VG_LOCK_ARGS_PATCH); VG_LOCK_ARGS_MAJOR, VG_LOCK_ARGS_MINOR, VG_LOCK_ARGS_PATCH);
/* see comment above about input vg_args being only lock_lv_name */ /* see comment above about input vg_args being only lock_lv_name */
(void)dm_strncpy(lock_lv_name, vg_args, sizeof(lock_lv_name)); dm_strncpy(lock_lv_name, vg_args, sizeof(lock_lv_name));
if (strlen(lock_lv_name) + strlen(lock_args_version) + 2 > MAX_ARGS) if (strlen(lock_lv_name) + strlen(lock_args_version) + 2 > MAX_ARGS)
return -EARGS; return -EARGS;
@ -573,7 +589,7 @@ int lm_init_vg_sanlock(char *ls_name, char *vg_name, uint32_t flags, char *vg_ar
} }
} }
(void)dm_strncpy(ss.name, ls_name, SANLK_NAME_LEN); strcpy_name_len(ss.name, ls_name, SANLK_NAME_LEN);
memcpy(ss.host_id_disk.path, disk.path, SANLK_PATH_LEN); memcpy(ss.host_id_disk.path, disk.path, SANLK_PATH_LEN);
ss.host_id_disk.offset = 0; ss.host_id_disk.offset = 0;
ss.flags = (sector_size == 4096) ? (SANLK_LSF_SECTOR4K | SANLK_LSF_ALIGN8M) : ss.flags = (sector_size == 4096) ? (SANLK_LSF_SECTOR4K | SANLK_LSF_ALIGN8M) :
@ -606,7 +622,7 @@ int lm_init_vg_sanlock(char *ls_name, char *vg_name, uint32_t flags, char *vg_ar
gl_name = R_NAME_GL; gl_name = R_NAME_GL;
memcpy(rd.rs.lockspace_name, ss.name, SANLK_NAME_LEN); memcpy(rd.rs.lockspace_name, ss.name, SANLK_NAME_LEN);
(void)dm_strncpy(rd.rs.name, gl_name, SANLK_NAME_LEN); strcpy_name_len(rd.rs.name, gl_name, SANLK_NAME_LEN);
memcpy(rd.rs.disks[0].path, disk.path, SANLK_PATH_LEN); memcpy(rd.rs.disks[0].path, disk.path, SANLK_PATH_LEN);
rd.rs.disks[0].offset = align_size * GL_LOCK_BEGIN; rd.rs.disks[0].offset = align_size * GL_LOCK_BEGIN;
rd.rs.num_disks = 1; rd.rs.num_disks = 1;
@ -621,7 +637,7 @@ int lm_init_vg_sanlock(char *ls_name, char *vg_name, uint32_t flags, char *vg_ar
} }
memcpy(rd.rs.lockspace_name, ss.name, SANLK_NAME_LEN); memcpy(rd.rs.lockspace_name, ss.name, SANLK_NAME_LEN);
(void)dm_strncpy(rd.rs.name, R_NAME_VG, SANLK_NAME_LEN); strcpy_name_len(rd.rs.name, R_NAME_VG, SANLK_NAME_LEN);
memcpy(rd.rs.disks[0].path, disk.path, SANLK_PATH_LEN); memcpy(rd.rs.disks[0].path, disk.path, SANLK_PATH_LEN);
rd.rs.disks[0].offset = align_size * VG_LOCK_BEGIN; rd.rs.disks[0].offset = align_size * VG_LOCK_BEGIN;
rd.rs.num_disks = 1; rd.rs.num_disks = 1;
@ -636,7 +652,7 @@ int lm_init_vg_sanlock(char *ls_name, char *vg_name, uint32_t flags, char *vg_ar
} }
if (!strcmp(gl_name, R_NAME_GL)) if (!strcmp(gl_name, R_NAME_GL))
(void)dm_strncpy(gl_lsname_sanlock, ls_name, MAX_NAME); dm_strncpy(gl_lsname_sanlock, ls_name, sizeof(gl_lsname_sanlock));
rv = snprintf(vg_args, MAX_ARGS, "%s:%s", lock_args_version, lock_lv_name); rv = snprintf(vg_args, MAX_ARGS, "%s:%s", lock_args_version, lock_lv_name);
if (rv >= MAX_ARGS) if (rv >= MAX_ARGS)
@ -655,8 +671,8 @@ int lm_init_vg_sanlock(char *ls_name, char *vg_name, uint32_t flags, char *vg_ar
rd.rs.flags = (sector_size == 4096) ? (SANLK_RES_SECTOR4K | SANLK_RES_ALIGN8M) : rd.rs.flags = (sector_size == 4096) ? (SANLK_RES_SECTOR4K | SANLK_RES_ALIGN8M) :
(SANLK_RES_SECTOR512 | SANLK_RES_ALIGN1M); (SANLK_RES_SECTOR512 | SANLK_RES_ALIGN1M);
memcpy(rd.rs.disks[0].path, disk.path, SANLK_PATH_LEN); memcpy(rd.rs.disks[0].path, disk.path, SANLK_PATH_LEN);
(void)dm_strncpy(rd.rs.lockspace_name, ls_name, SANLK_NAME_LEN); strcpy_name_len(rd.rs.lockspace_name, ls_name, SANLK_NAME_LEN);
(void)dm_strncpy(rd.rs.name, "#unused", SANLK_NAME_LEN); strcpy_name_len(rd.rs.name, "#unused", SANLK_NAME_LEN);
offset = align_size * LV_LOCK_BEGIN; offset = align_size * LV_LOCK_BEGIN;
@ -724,7 +740,7 @@ int lm_init_lv_sanlock(char *ls_name, char *vg_name, char *lv_name,
return 0; return 0;
} }
(void)dm_strncpy(rd.rs.lockspace_name, ls_name, SANLK_NAME_LEN); strcpy_name_len(rd.rs.lockspace_name, ls_name, SANLK_NAME_LEN);
rd.rs.num_disks = 1; rd.rs.num_disks = 1;
if ((rv = build_dm_path(rd.rs.disks[0].path, SANLK_PATH_LEN, vg_name, lock_lv_name))) if ((rv = build_dm_path(rd.rs.disks[0].path, SANLK_PATH_LEN, vg_name, lock_lv_name)))
return rv; return rv;
@ -799,7 +815,7 @@ int lm_init_lv_sanlock(char *ls_name, char *vg_name, char *lv_name,
log_debug("S %s init_lv_san %s found unused area at %llu", log_debug("S %s init_lv_san %s found unused area at %llu",
ls_name, lv_name, (unsigned long long)offset); ls_name, lv_name, (unsigned long long)offset);
(void)dm_strncpy(rd.rs.name, lv_name, SANLK_NAME_LEN); strcpy_name_len(rd.rs.name, lv_name, SANLK_NAME_LEN);
rd.rs.flags = (sector_size == 4096) ? (SANLK_RES_SECTOR4K | SANLK_RES_ALIGN8M) : rd.rs.flags = (sector_size == 4096) ? (SANLK_RES_SECTOR4K | SANLK_RES_ALIGN8M) :
(SANLK_RES_SECTOR512 | SANLK_RES_ALIGN1M); (SANLK_RES_SECTOR512 | SANLK_RES_ALIGN1M);
@ -898,7 +914,7 @@ int lm_rename_vg_sanlock(char *ls_name, char *vg_name, uint32_t flags, char *vg_
if (!sector_size || !align_size) if (!sector_size || !align_size)
return -1; return -1;
(void)dm_strncpy(ss.name, ls_name, SANLK_NAME_LEN); strcpy_name_len(ss.name, ls_name, SANLK_NAME_LEN);
rv = sanlock_write_lockspace(&ss, 0, 0, sanlock_io_timeout); rv = sanlock_write_lockspace(&ss, 0, 0, sanlock_io_timeout);
if (rv < 0) { if (rv < 0) {
@ -1008,7 +1024,7 @@ int lm_free_lv_sanlock(struct lockspace *ls, struct resource *r)
if (daemon_test) if (daemon_test)
return 0; return 0;
(void)dm_strncpy(rs->name, "#unused", SANLK_NAME_LEN); strcpy_name_len(rs->name, "#unused", SANLK_NAME_LEN);
rv = sanlock_write_resource(rs, 0, 0, 0); rv = sanlock_write_resource(rs, 0, 0, 0);
if (rv < 0) { if (rv < 0) {
@ -1042,11 +1058,11 @@ int lm_ex_disable_gl_sanlock(struct lockspace *ls)
memset(&rd1, 0, sizeof(rd1)); memset(&rd1, 0, sizeof(rd1));
memset(&rd2, 0, sizeof(rd2)); memset(&rd2, 0, sizeof(rd2));
(void)dm_strncpy(rd1.rs.lockspace_name, ls->name, SANLK_NAME_LEN); strcpy_name_len(rd1.rs.lockspace_name, ls->name, SANLK_NAME_LEN);
(void)dm_strncpy(rd1.rs.name, R_NAME_GL, SANLK_NAME_LEN); strcpy_name_len(rd1.rs.name, R_NAME_GL, SANLK_NAME_LEN);
(void)dm_strncpy(rd2.rs.lockspace_name, ls->name, SANLK_NAME_LEN); strcpy_name_len(rd2.rs.lockspace_name, ls->name, SANLK_NAME_LEN);
(void)dm_strncpy(rd2.rs.name, R_NAME_GL_DISABLED, SANLK_NAME_LEN); strcpy_name_len(rd2.rs.name, R_NAME_GL_DISABLED, SANLK_NAME_LEN);
rd1.rs.num_disks = 1; rd1.rs.num_disks = 1;
memcpy(rd1.rs.disks[0].path, lms->ss.host_id_disk.path, SANLK_PATH_LEN-1); memcpy(rd1.rs.disks[0].path, lms->ss.host_id_disk.path, SANLK_PATH_LEN-1);
@ -1111,8 +1127,8 @@ int lm_able_gl_sanlock(struct lockspace *ls, int enable)
memset(&rd, 0, sizeof(rd)); memset(&rd, 0, sizeof(rd));
(void)dm_strncpy(rd.rs.lockspace_name, ls->name, SANLK_NAME_LEN); strcpy_name_len(rd.rs.lockspace_name, ls->name, SANLK_NAME_LEN);
(void)dm_strncpy(rd.rs.name, gl_name, SANLK_NAME_LEN); strcpy_name_len(rd.rs.name, gl_name, SANLK_NAME_LEN);
rd.rs.num_disks = 1; rd.rs.num_disks = 1;
memcpy(rd.rs.disks[0].path, lms->ss.host_id_disk.path, SANLK_PATH_LEN-1); memcpy(rd.rs.disks[0].path, lms->ss.host_id_disk.path, SANLK_PATH_LEN-1);
@ -1132,7 +1148,7 @@ out:
ls->sanlock_gl_enabled = enable; ls->sanlock_gl_enabled = enable;
if (enable) if (enable)
(void)dm_strncpy(gl_lsname_sanlock, ls->name, MAX_NAME); dm_strncpy(gl_lsname_sanlock, ls->name, sizeof(gl_lsname_sanlock));
if (!enable && !strcmp(gl_lsname_sanlock, ls->name)) if (!enable && !strcmp(gl_lsname_sanlock, ls->name))
memset(gl_lsname_sanlock, 0, sizeof(gl_lsname_sanlock)); memset(gl_lsname_sanlock, 0, sizeof(gl_lsname_sanlock));
@ -1152,7 +1168,7 @@ static int gl_is_enabled(struct lockspace *ls, struct lm_sanlock *lms)
memset(&rd, 0, sizeof(rd)); memset(&rd, 0, sizeof(rd));
(void)dm_strncpy(rd.rs.lockspace_name, ls->name, SANLK_NAME_LEN); strcpy_name_len(rd.rs.lockspace_name, ls->name, SANLK_NAME_LEN);
/* leave rs.name empty, it is what we're checking */ /* leave rs.name empty, it is what we're checking */
@ -1223,7 +1239,7 @@ int lm_find_free_lock_sanlock(struct lockspace *ls, uint64_t *free_offset, int *
memset(&rd, 0, sizeof(rd)); memset(&rd, 0, sizeof(rd));
(void)dm_strncpy(rd.rs.lockspace_name, ls->name, SANLK_NAME_LEN); strcpy_name_len(rd.rs.lockspace_name, ls->name, SANLK_NAME_LEN);
rd.rs.num_disks = 1; rd.rs.num_disks = 1;
memcpy(rd.rs.disks[0].path, lms->ss.host_id_disk.path, SANLK_PATH_LEN-1); memcpy(rd.rs.disks[0].path, lms->ss.host_id_disk.path, SANLK_PATH_LEN-1);
rd.rs.flags = (lms->sector_size == 4096) ? (SANLK_RES_SECTOR4K | SANLK_RES_ALIGN8M) : rd.rs.flags = (lms->sector_size == 4096) ? (SANLK_RES_SECTOR4K | SANLK_RES_ALIGN8M) :
@ -1574,7 +1590,7 @@ int lm_rem_lockspace_sanlock(struct lockspace *ls, int free_vg)
* This shouldn't be generally necessary, but there may some races * This shouldn't be generally necessary, but there may some races
* between nodes starting and removing a vg which this could help. * between nodes starting and removing a vg which this could help.
*/ */
(void)dm_strncpy(lms->ss.name, "#unused", SANLK_NAME_LEN); strcpy_name_len(lms->ss.name, "#unused", SANLK_NAME_LEN);
rv = sanlock_write_lockspace(&lms->ss, 0, 0, sanlock_io_timeout); rv = sanlock_write_lockspace(&lms->ss, 0, 0, sanlock_io_timeout);
if (rv < 0) { if (rv < 0) {
@ -1602,8 +1618,8 @@ static int lm_add_resource_sanlock(struct lockspace *ls, struct resource *r)
struct lm_sanlock *lms = (struct lm_sanlock *)ls->lm_data; struct lm_sanlock *lms = (struct lm_sanlock *)ls->lm_data;
struct rd_sanlock *rds = (struct rd_sanlock *)r->lm_data; struct rd_sanlock *rds = (struct rd_sanlock *)r->lm_data;
(void)dm_strncpy(rds->rs.lockspace_name, ls->name, SANLK_NAME_LEN); strcpy_name_len(rds->rs.lockspace_name, ls->name, SANLK_NAME_LEN);
(void)dm_strncpy(rds->rs.name, r->name, SANLK_NAME_LEN); strcpy_name_len(rds->rs.name, r->name, SANLK_NAME_LEN);
rds->rs.num_disks = 1; rds->rs.num_disks = 1;
memcpy(rds->rs.disks[0].path, lms->ss.host_id_disk.path, SANLK_PATH_LEN); memcpy(rds->rs.disks[0].path, lms->ss.host_id_disk.path, SANLK_PATH_LEN);
rds->rs.flags = (lms->sector_size == 4096) ? (SANLK_RES_SECTOR4K | SANLK_RES_ALIGN8M) : (SANLK_RES_SECTOR512 | SANLK_RES_ALIGN1M); rds->rs.flags = (lms->sector_size == 4096) ? (SANLK_RES_SECTOR4K | SANLK_RES_ALIGN8M) : (SANLK_RES_SECTOR512 | SANLK_RES_ALIGN1M);
@ -2032,7 +2048,7 @@ static int release_rename(struct lockspace *ls, struct resource *r)
res1 = (struct sanlk_resource *)&rd1; res1 = (struct sanlk_resource *)&rd1;
res2 = (struct sanlk_resource *)&rd2; res2 = (struct sanlk_resource *)&rd2;
(void)dm_strncpy(res2->name, "invalid_removed", SANLK_NAME_LEN); strcpy_name_len(res2->name, "invalid_removed", SANLK_NAME_LEN);
res_args[0] = res1; res_args[0] = res1;
res_args[1] = res2; res_args[1] = res2;