On (25/02/23 14:55), Herbert Xu wrote:
[..]
> } else if (m_off <= M3_MAX_OFFSET) {
> m_off -= 1;
> + if (!HAVE_OP(1))
> + return LZO_E_OUTPUT_OVERRUN;
> if (m_len <= M3_MAX_LEN)
> *op++ = (M3_MARKER | (m_len - 2));
> else {
[..]
> } else {
> m_off -= 0x4000;
> + if (!HAVE_OP(1))
> + return LZO_E_OUTPUT_OVERRUN;
> if (m_len <= M4_MAX_LEN)
> *op++ = (M4_MARKER | ((m_off >> 11) & 8)
> | (m_len - 2));
Made me wonder if HAVE_OP() in these two cases should have been
under if, but it covers both if and else branches, so it's all
right.
[..]
> +++ b/lib/lzo/lzo1x_decompress_safe.c
> @@ -21,7 +21,6 @@
> #include "lzodefs.h"
>
> #define HAVE_IP(x) ((size_t)(ip_end - ip) >= (size_t)(x))
> -#define HAVE_OP(x) ((size_t)(op_end - op) >= (size_t)(x))
> #define NEED_IP(x) if (!HAVE_IP(x)) goto input_overrun
> #define NEED_OP(x) if (!HAVE_OP(x)) goto output_overrun
A bit of a pity that NEED_OP() with "goto output_overrun" is only
for decompression. It looks equally relevant to the compression
path. I'm not insisting on doing something similar in compression
tho.
Overall this look right, and kudos to Herbert for doing this.
I did some testing (using my usual zram test scripts, but modified
zram to allocate only one physical page for comp buf and to check
for overrun ret status). Haven't noticed any problems.
FWIW
Reviewed-and-tested-by: Sergey Senozhatsky <senozhatsky@xxxxxxxxxxxx>
