Skip to content

Commit 6f95a50

Browse files
cujomalaineycarlescufi
authored andcommitted
lib: fix ubsan errors in cbvprintf_package
It is undefined behaviour to shift / add offsets to a null pointer. Move to direct offset tracking to satisfy UBSAN. Simple translation of code: buf0 -> buf buf +=/++ -> offset +=/++ buf = -> buf+offset = Signed-off-by: Curtis Malainey <[email protected]>
1 parent d056455 commit 6f95a50

File tree

1 file changed

+53
-52
lines changed

1 file changed

+53
-52
lines changed

lib/os/cbprintf_packaged.c

Lines changed: 53 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -242,10 +242,10 @@ int cbvprintf_package(void *packaged, size_t len, uint32_t flags,
242242
#define STR_POS_MASK BIT_MASK(7)
243243

244244
/* Buffer offset abstraction for better code clarity. */
245-
#define BUF_OFFSET ((uintptr_t)buf - (uintptr_t)buf0)
245+
#define BUF_OFFSET offset
246246

247-
uint8_t *buf0 = packaged; /* buffer start (may be NULL) */
248-
uint8_t *buf = buf0; /* current buffer position */
247+
uint8_t *buf = packaged; /* buffer start (may be NULL) */
248+
size_t offset = 0; /* current buffer position */
249249
unsigned int size; /* current argument's size */
250250
unsigned int align; /* current argument's required alignment */
251251
uint8_t str_ptr_pos[16]; /* string pointer positions */
@@ -294,16 +294,16 @@ int cbvprintf_package(void *packaged, size_t len, uint32_t flags,
294294
*
295295
* Refer to union cbprintf_package_hdr for more details.
296296
*/
297-
buf += sizeof(*pkg_hdr);
297+
offset += sizeof(*pkg_hdr);
298298

299299
/*
300-
* When buf0 is NULL we don't store anything.
300+
* When buf is NULL we don't store anything.
301301
* Instead we count the needed space to store the data.
302302
* In this case, incoming len argument indicates the anticipated
303303
* buffer "misalignment" offset.
304304
*/
305-
if (buf0 == NULL) {
306-
buf += len % CBPRINTF_PACKAGE_ALIGNMENT;
305+
if (buf == NULL) {
306+
offset += len % CBPRINTF_PACKAGE_ALIGNMENT;
307307
/*
308308
* The space to store the data is represented by both the
309309
* buffer offset as well as the extra string data to be
@@ -324,7 +324,7 @@ int cbvprintf_package(void *packaged, size_t len, uint32_t flags,
324324
* Otherwise we must ensure we can store at least
325325
* the pointer to the format string itself.
326326
*/
327-
if ((buf0 != NULL) && (BUF_OFFSET + sizeof(char *)) > len) {
327+
if ((buf != NULL) && (BUF_OFFSET + sizeof(char *)) > len) {
328328
return -ENOSPC;
329329
}
330330

@@ -355,18 +355,18 @@ int cbvprintf_package(void *packaged, size_t len, uint32_t flags,
355355
size = sizeof(int);
356356

357357
/* align destination buffer location */
358-
buf = (void *)ROUND_UP(buf, align);
358+
offset = ROUND_UP(offset, align);
359359

360360
/* make sure the data fits */
361-
if (buf0 != NULL && BUF_OFFSET + size > len) {
361+
if (buf != NULL && BUF_OFFSET + size > len) {
362362
return -ENOSPC;
363363
}
364364

365-
if (buf0 != NULL) {
366-
*(int *)buf = arg_tag;
365+
if (buf != NULL) {
366+
*(int *)(buf + offset) = arg_tag;
367367
}
368368

369-
buf += sizeof(int);
369+
offset += sizeof(int);
370370

371371
if (arg_tag == CBPRINTF_PACKAGE_ARG_TYPE_END) {
372372
/* End of arguments */
@@ -430,21 +430,21 @@ int cbvprintf_package(void *packaged, size_t len, uint32_t flags,
430430
}
431431

432432
/* align destination buffer location */
433-
buf = (void *) ROUND_UP(buf, align);
434-
if (buf0 != NULL) {
433+
offset = ROUND_UP(offset, align);
434+
if (buf != NULL) {
435435
/* make sure it fits */
436436
if ((BUF_OFFSET + size) > len) {
437437
return -ENOSPC;
438438
}
439439
if (Z_CBPRINTF_VA_STACK_LL_DBL_MEMCPY) {
440-
memcpy(buf, (uint8_t *)&v, size);
440+
memcpy((buf + offset), (uint8_t *)&v, size);
441441
} else if (fmt[-1] == 'L') {
442-
*(long double *)buf = v.ld;
442+
*(long double *)(buf + offset) = v.ld;
443443
} else {
444-
*(double *)buf = v.d;
444+
*(double *)(buf + offset) = v.d;
445445
}
446446
}
447-
buf += size;
447+
offset += size;
448448
parsing = false;
449449
continue;
450450
}
@@ -577,21 +577,21 @@ int cbvprintf_package(void *packaged, size_t len, uint32_t flags,
577577
size = sizeof(double);
578578
}
579579
/* align destination buffer location */
580-
buf = (void *) ROUND_UP(buf, align);
581-
if (buf0 != NULL) {
580+
offset = ROUND_UP(offset, align);
581+
if (buf != NULL) {
582582
/* make sure it fits */
583583
if (BUF_OFFSET + size > len) {
584584
return -ENOSPC;
585585
}
586586
if (Z_CBPRINTF_VA_STACK_LL_DBL_MEMCPY) {
587-
memcpy(buf, (uint8_t *)&v, size);
587+
memcpy(buf + offset, (uint8_t *)&v, size);
588588
} else if (fmt[-1] == 'L') {
589-
*(long double *)buf = v.ld;
589+
*(long double *)(buf + offset) = v.ld;
590590
} else {
591-
*(double *)buf = v.d;
591+
*(double *)(buf + offset) = v.d;
592592
}
593593
}
594-
buf += size;
594+
offset += size;
595595
parsing = false;
596596
continue;
597597
}
@@ -603,19 +603,19 @@ int cbvprintf_package(void *packaged, size_t len, uint32_t flags,
603603
}
604604

605605
/* align destination buffer location */
606-
buf = (void *) ROUND_UP(buf, align);
606+
offset = ROUND_UP(offset, align);
607607

608608
/* make sure the data fits */
609-
if ((buf0 != NULL) && (BUF_OFFSET + size) > len) {
609+
if ((buf != NULL) && (BUF_OFFSET + size) > len) {
610610
return -ENOSPC;
611611
}
612612

613613
/* copy va_list data over to our buffer */
614614
if (is_str_arg) {
615615
s = va_arg(ap, char *);
616616
process_string:
617-
if (buf0 != NULL) {
618-
*(const char **)buf = s;
617+
if (buf != NULL) {
618+
*(const char **)(buf + offset) = s;
619619
}
620620

621621
bool is_ro = (fros_cnt-- > 0) ? true : ptr_in_rodata(s);
@@ -642,7 +642,7 @@ int cbvprintf_package(void *packaged, size_t len, uint32_t flags,
642642
return -EINVAL;
643643
}
644644

645-
if (buf0 != NULL) {
645+
if (buf != NULL) {
646646
/*
647647
* Remember string pointer location.
648648
* We will append non-ro strings later.
@@ -678,34 +678,34 @@ int cbvprintf_package(void *packaged, size_t len, uint32_t flags,
678678

679679
s_idx++;
680680
}
681-
buf += sizeof(char *);
681+
offset += sizeof(char *);
682682

683683
is_str_arg = false;
684684
} else if (size == sizeof(int)) {
685685
int v = va_arg(ap, int);
686686

687-
if (buf0 != NULL) {
688-
*(int *)buf = v;
687+
if (buf != NULL) {
688+
*(int *)(buf + offset) = v;
689689
}
690-
buf += sizeof(int);
690+
offset += sizeof(int);
691691
} else if (size == sizeof(long)) {
692692
long v = va_arg(ap, long);
693693

694-
if (buf0 != NULL) {
695-
*(long *)buf = v;
694+
if (buf != NULL) {
695+
*(long *)(buf + offset) = v;
696696
}
697-
buf += sizeof(long);
697+
offset += sizeof(long);
698698
} else if (size == sizeof(long long)) {
699699
long long v = va_arg(ap, long long);
700700

701-
if (buf0 != NULL) {
701+
if (buf != NULL) {
702702
if (Z_CBPRINTF_VA_STACK_LL_DBL_MEMCPY) {
703-
memcpy(buf, (uint8_t *)&v, sizeof(long long));
703+
memcpy(buf + offset, (uint8_t *)&v, sizeof(long long));
704704
} else {
705-
*(long long *)buf = v;
705+
*(long long *)(buf + offset) = v;
706706
}
707707
}
708-
buf += sizeof(long long);
708+
offset += sizeof(long long);
709709
} else {
710710
__ASSERT(false, "unexpected size %u", size);
711711
return -EINVAL;
@@ -727,12 +727,12 @@ int cbvprintf_package(void *packaged, size_t len, uint32_t flags,
727727
* If all we wanted was to count required buffer size
728728
* then we have it now.
729729
*/
730-
if (buf0 == NULL) {
730+
if (buf == NULL) {
731731
return BUF_OFFSET + len - CBPRINTF_PACKAGE_ALIGNMENT;
732732
}
733733

734734
/* Clear our buffer header. We made room for it initially. */
735-
*(char **)buf0 = NULL;
735+
*(char **)buf = NULL;
736736

737737
/* Record end of argument list. */
738738
pkg_hdr->desc.len = BUF_OFFSET / sizeof(int);
@@ -767,8 +767,8 @@ int cbvprintf_package(void *packaged, size_t len, uint32_t flags,
767767
return -ENOSPC;
768768
}
769769
/* store the pointer position prefix */
770-
*buf = pos;
771-
++buf;
770+
*(buf + offset) = pos;
771+
++offset;
772772
}
773773
}
774774

@@ -781,12 +781,13 @@ int cbvprintf_package(void *packaged, size_t len, uint32_t flags,
781781

782782
if (rws_pos_en) {
783783
size = 0;
784-
*buf++ = str_ptr_arg[i];
784+
*(buf + offset) = str_ptr_arg[i];
785+
offset++;
785786
} else {
786787
/* retrieve the string pointer */
787-
s = *(char **)(buf0 + str_ptr_pos[i] * sizeof(int));
788+
s = *(char **)(buf + str_ptr_pos[i] * sizeof(int));
788789
/* clear the in-buffer pointer (less entropy if compressed) */
789-
*(char **)(buf0 + str_ptr_pos[i] * sizeof(int)) = NULL;
790+
*(char **)(buf + str_ptr_pos[i] * sizeof(int)) = NULL;
790791
/* find the string length including terminating '\0' */
791792
size = strlen(s) + 1;
792793
}
@@ -796,11 +797,11 @@ int cbvprintf_package(void *packaged, size_t len, uint32_t flags,
796797
return -ENOSPC;
797798
}
798799
/* store the pointer position prefix */
799-
*buf = str_ptr_pos[i];
800-
++buf;
800+
*(buf + offset) = str_ptr_pos[i];
801+
++offset;
801802
/* copy the string with its terminating '\0' */
802-
memcpy(buf, (uint8_t *)s, size);
803-
buf += size;
803+
memcpy(buf + offset, (uint8_t *)s, size);
804+
offset += size;
804805
}
805806

806807
/*

0 commit comments

Comments
 (0)