On 11.12.2017 23:19, Collin L. Walling wrote:
> Moved:
> memcmp from bootmap.h to libc.h (renamed from _memcmp)
> strlen from sclp.c to libc.h (renamed from _strlen)
>
> Added C standard functions:
> isdigit
> atoi
>
> Added non-C standard function:
> itostr
>
> Signed-off-by: Collin L. Walling <wall...@linux.vnet.ibm.com>
> Acked-by: Christian Borntraeger <borntrae...@de.ibm.com>
> Reviewed-by: Janosch Frank <fran...@linux.vnet.ibm.com>
> ---
> pc-bios/s390-ccw/Makefile | 2 +-
> pc-bios/s390-ccw/bootmap.c | 4 +--
> pc-bios/s390-ccw/bootmap.h | 16 +---------
> pc-bios/s390-ccw/libc.c | 75
> ++++++++++++++++++++++++++++++++++++++++++++++
> pc-bios/s390-ccw/libc.h | 31 +++++++++++++++++++
> pc-bios/s390-ccw/main.c | 17 +----------
> pc-bios/s390-ccw/sclp.c | 10 +------
> 7 files changed, 112 insertions(+), 43 deletions(-)
> create mode 100644 pc-bios/s390-ccw/libc.c
[...]
> +
> +/**
> + * itostr:
> + * @num: the integer to be converted.
> + * @str: a pointer to a string to store the conversion.
> + * @len: the length of the passed string.
> + *
> + * Given an integer @num, convert it to a string. The string @str must be
> + * allocated beforehand. The resulting string will be null terminated and
> + * returned.
> + *
> + * Returns: the string @str of the converted integer @num.
> + */
> +char *itostr(int num, char *str, size_t len)
> +{
> + long num_len = 1;
> + int tmp = num;
> + int i;
> +
> + /* Count length of num */
> + while ((tmp /= 10) > 0) {
> + num_len++;
> + }
> +
> + /* Check if we have enough space for num and null */
> + if (len < num_len) {
> + return 0;
> + }
I'm afraid, but I think you've got an off-by-one bug in this code.
In patch 5, you're using this function like this:
char tmp[4];
sclp_print(itostr(entries, tmp, sizeof(tmp)));
That means if entries is >= 1000 for example, num_len is 4 ...
> + /* Convert int to string */
> + for (i = num_len - 1; i >= 0; i--) {
> + str[i] = num % 10 + '0';
> + num /= 10;
> + }
> +
> + str[num_len] = '\0';
... and then you run into a buffer overflow here.
> + return str;
> +}
Maybe it would also make more sense to panic() instead of "return 0"
since you don't check the return value in patch 5 ?
Thomas
