Skip to content

Commit 7ae14eb

Browse files
rghaddabkartben
authored andcommitted
settings: zms: use the safe function strnlen instead of strlen
if the provided name in argument is not null this could lead to un undefined behavior. Use strnlen to make this safe Signed-off-by: Riadh Ghaddab <rghaddab@baylibre.com>
1 parent cad2f1c commit 7ae14eb

File tree

2 files changed

+30
-11
lines changed

2 files changed

+30
-11
lines changed

include/zephyr/settings/settings.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,9 @@ extern "C" {
4545
*/
4646
#define SETTINGS_EXTRA_LEN ((SETTINGS_MAX_DIR_DEPTH - 1) + 2)
4747

48+
/* Maximum Settings name length including separators */
49+
#define SETTINGS_FULL_NAME_LEN ((SETTINGS_MAX_NAME_LEN) + (SETTINGS_EXTRA_LEN) + 1)
50+
4851
/**
4952
* Function used to read the data from the settings storage in
5053
* h_set handler implementations.

subsys/settings/src/settings_zms.c

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@
33
* SPDX-License-Identifier: Apache-2.0
44
*/
55

6+
#undef _POSIX_C_SOURCE
7+
#define _POSIX_C_SOURCE 200809L /* for strnlen() */
8+
69
#include <errno.h>
710
#include <string.h>
811

@@ -169,12 +172,13 @@ static int settings_zms_load_subtree(struct settings_store *cs, const struct set
169172
{
170173
struct settings_zms *cf = CONTAINER_OF(cs, struct settings_zms, cf_store);
171174
struct settings_zms_read_fn_arg read_fn_arg;
172-
char name[SETTINGS_MAX_NAME_LEN + SETTINGS_EXTRA_LEN + 1];
175+
char name[SETTINGS_FULL_NAME_LEN];
173176
ssize_t rc1;
174177
ssize_t rc2;
175178
uint32_t name_hash;
179+
size_t name_len = strnlen(arg->subtree, SETTINGS_FULL_NAME_LEN);
176180

177-
name_hash = sys_hash32(arg->subtree, strlen(arg->subtree)) & ZMS_HASH_MASK;
181+
name_hash = sys_hash32(arg->subtree, name_len) & ZMS_HASH_MASK;
178182
for (int i = 0; i <= cf->hash_collision_num; i++) {
179183
name_hash = ZMS_UPDATE_COLLISION_NUM(name_hash, i);
180184
/* Get the name entry from ZMS */
@@ -214,17 +218,21 @@ static ssize_t settings_zms_load_one(struct settings_store *cs, const char *name
214218
size_t buf_len)
215219
{
216220
struct settings_zms *cf = CONTAINER_OF(cs, struct settings_zms, cf_store);
217-
char r_name[SETTINGS_MAX_NAME_LEN + SETTINGS_EXTRA_LEN + 1];
221+
char r_name[SETTINGS_FULL_NAME_LEN];
218222
ssize_t rc = 0;
219223
uint32_t name_hash;
220224
uint32_t value_id;
225+
size_t name_len;
221226

222227
/* verify that name is not NULL */
223228
if (!name || !buf) {
224229
return -EINVAL;
225230
}
226231

227-
name_hash = sys_hash32(name, strlen(name)) & ZMS_HASH_MASK;
232+
/* get the name length */
233+
name_len = strnlen(name, SETTINGS_FULL_NAME_LEN);
234+
235+
name_hash = sys_hash32(name, name_len) & ZMS_HASH_MASK;
228236
for (int i = 0; i <= cf->hash_collision_num; i++) {
229237
name_hash = ZMS_UPDATE_COLLISION_NUM(name_hash, i);
230238
/* Get the name entry from ZMS */
@@ -260,7 +268,7 @@ static int settings_zms_load(struct settings_store *cs, const struct settings_lo
260268
struct settings_zms *cf = CONTAINER_OF(cs, struct settings_zms, cf_store);
261269
struct settings_zms_read_fn_arg read_fn_arg;
262270
struct settings_hash_linked_list settings_element;
263-
char name[SETTINGS_MAX_NAME_LEN + SETTINGS_EXTRA_LEN + 1];
271+
char name[SETTINGS_FULL_NAME_LEN];
264272
ssize_t rc1;
265273
ssize_t rc2;
266274
uint32_t ll_hash_id;
@@ -381,23 +389,27 @@ static int settings_zms_save(struct settings_store *cs, const char *name, const
381389
{
382390
struct settings_zms *cf = CONTAINER_OF(cs, struct settings_zms, cf_store);
383391
struct settings_hash_linked_list settings_element;
384-
char rdname[SETTINGS_MAX_NAME_LEN + SETTINGS_EXTRA_LEN + 1];
392+
char rdname[SETTINGS_FULL_NAME_LEN];
385393
uint32_t name_hash;
386394
uint32_t collision_num = 0;
387395
bool delete;
388396
bool write_name;
389397
bool hash_collision;
390398
int rc = 0;
391399
int first_available_hash_index = -1;
400+
size_t name_len;
392401

393402
if (!name) {
394403
return -EINVAL;
395404
}
396405

406+
/* get the name length */
407+
name_len = strnlen(name, SETTINGS_FULL_NAME_LEN);
408+
397409
/* Find out if we are doing a delete */
398410
delete = ((value == NULL) || (val_len == 0));
399411

400-
name_hash = sys_hash32(name, strlen(name)) & ZMS_HASH_MASK;
412+
name_hash = sys_hash32(name, name_len) & ZMS_HASH_MASK;
401413
/* MSB is always 1 */
402414
name_hash |= BIT(31);
403415

@@ -421,7 +433,7 @@ static int settings_zms_save(struct settings_store *cs, const char *name, const
421433
* name
422434
*/
423435
rdname[rc] = '\0';
424-
if (!strcmp(name, rdname)) {
436+
if ((rc == name_len) && !memcmp(name, rdname, rc)) {
425437
/* Hash exist and the names are equal, we should
426438
* not write the names again.
427439
*/
@@ -524,7 +536,7 @@ static int settings_zms_save(struct settings_store *cs, const char *name, const
524536
no_ll_update:
525537
#endif /* CONFIG_SETTINGS_ZMS_NO_LL_DELETE */
526538
/* Now let's write the name */
527-
rc = zms_write(&cf->cf_zms, name_hash, name, strlen(name));
539+
rc = zms_write(&cf->cf_zms, name_hash, name, name_len);
528540
if (rc < 0) {
529541
return rc;
530542
}
@@ -535,16 +547,20 @@ static int settings_zms_save(struct settings_store *cs, const char *name, const
535547
static ssize_t settings_zms_get_val_len(struct settings_store *cs, const char *name)
536548
{
537549
struct settings_zms *cf = CONTAINER_OF(cs, struct settings_zms, cf_store);
538-
char r_name[SETTINGS_MAX_NAME_LEN + SETTINGS_EXTRA_LEN + 1];
550+
char r_name[SETTINGS_FULL_NAME_LEN];
539551
ssize_t rc = 0;
540552
uint32_t name_hash;
553+
size_t name_len;
541554

542555
/* verify that name is not NULL */
543556
if (!name) {
544557
return -EINVAL;
545558
}
546559

547-
name_hash = sys_hash32(name, strlen(name)) & ZMS_HASH_MASK;
560+
/* get the name length */
561+
name_len = strnlen(name, SETTINGS_FULL_NAME_LEN);
562+
563+
name_hash = sys_hash32(name, name_len) & ZMS_HASH_MASK;
548564
for (int i = 0; i <= cf->hash_collision_num; i++) {
549565
name_hash = ZMS_UPDATE_COLLISION_NUM(name_hash, i);
550566
/* Get the name entry from ZMS */

0 commit comments

Comments
 (0)