On Thu, Mar 2, 2023 at 2:58 PM Kees Cook <keescook@xxxxxxxxxxxx> wrote:
>
> diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h
> index c9de1f59ee80..981e2838f99a 100644
> --- a/include/linux/fortify-string.h
> +++ b/include/linux/fortify-string.h
> @@ -170,11 +170,13 @@ __FORTIFY_INLINE __diagnose_as(__builtin_strcat, 1, 2)
> char *strcat(char * const POS p, const char *q)
> {
> size_t p_size = __member_size(p);
> + size_t size;
>
> if (p_size == SIZE_MAX)
> return __underlying_strcat(p, q);
> - if (strlcat(p, q, p_size) >= p_size)
> - fortify_panic(__func__);
> + size = strlcat(p, q, p_size);
> + if (p_size < size)
What happens when they're equal? I think this patch changes
behavior...? Intentional?
Did flipping this conditional drop what should be `<=`?
Was there an off by one, or is this version of this patch potentially
introducing one? Or am I misremembering my boolean algebra?
> + fortify_panic(__func__, 1, p_size, size);
> return p;
> }
>
> @@ -205,7 +207,7 @@ __FORTIFY_INLINE __kernel_size_t strnlen(const char * const POS p, __kernel_size
> /* Do not check characters beyond the end of p. */
> ret = __real_strnlen(p, maxlen < p_size ? maxlen : p_size);
> if (p_size <= ret && maxlen != ret)
> - fortify_panic(__func__);
> + fortify_panic(__func__, 1, p_size, ret);
> return ret;
> }
>
> @@ -241,7 +243,7 @@ __kernel_size_t __fortify_strlen(const char * const POS p)
> return __underlying_strlen(p);
> ret = strnlen(p, p_size);
> if (p_size <= ret)
> - fortify_panic(__func__);
> + fortify_panic(__func__, 1, p_size, ret);
> return ret;
> }
>
> @@ -282,8 +284,8 @@ __FORTIFY_INLINE size_t strlcpy(char * const POS p, const char * const POS q, si
> __write_overflow();
> }
> if (size) {
> - if (len >= p_size)
> - fortify_panic(__func__);
> + if (p_size < len)
`<=` ? (This used to panic when they were equal)
> + fortify_panic(__func__, 1, p_size, len);
> __underlying_memcpy(p, q, len);
> p[len] = '\0';
> }
--
Thanks,
~Nick Desaulniers
