[Tarantool-patches] [PATCH v1 1/1] sql: fix a segfault in hex() on receiving zeroblob
Safin Timur
tsafin at tarantool.org
Fri Sep 3 22:20:01 MSK 2021
This version is much simpler and is quite readable as is.
LGTM.
Though few unimportant notes...
On 30.08.2021 9:20, imeevma at tarantool.org wrote:
> This patch fixes a segmentation fault when zeroblob is received by the
> SQL built-in HEX() function.
>
> Closes #6113
> ---
> https://github.com/tarantool/tarantool/issues/6113
> https://github.com/tarantool/tarantool/tree/imeevma/gh-6113-fix-hex-segfault-2.8
>
> diff --git a/src/box/sql/func.c b/src/box/sql/func.c
> index b137c6125..3ef31705e 100644
> --- a/src/box/sql/func.c
> +++ b/src/box/sql/func.c
> @@ -1221,15 +1221,21 @@ hexFunc(sql_context * context, int argc, sql_value ** argv)
> UNUSED_PARAMETER(argc);
> pBlob = mem_as_bin(argv[0]);
> n = mem_len_unsafe(argv[0]);
> + assert((argv[0]->flags & MEM_Zero) == 0 ||
> + argv[0]->type == MEM_TYPE_BIN);
I believe this is unncessary, as those exactly checks were already done
inside of mem_len()
> + int zero_len = (argv[0]->flags & MEM_Zero) == 0 ? 0 : argv[0]->u.nZero;
> assert(pBlob == mem_as_bin(argv[0])); /* No encoding change */
> z = zHex = contextMalloc(context, ((i64) n) * 2 + 1);
Worth to note that here contextMalloc() used to check passed length
against SQL_LIMIT_LENGTH, in the newer code this check disappeared.
> if (zHex) {
> - for (i = 0; i < n; i++, pBlob++) {
> + for (i = 0; i < n - zero_len; i++, pBlob++) {
> unsigned char c = *pBlob;
> *(z++) = hexdigits[(c >> 4) & 0xf];
> *(z++) = hexdigits[c & 0xf];
> }
> - *z = 0;
> + assert(i == n || (argv[0]->flags & MEM_Zero) != 0);
> + assert(n == zero_len + i);
> + memset(z, '0', 2 * zero_len);
> + z[2 * zero_len] = '\0';
> sql_result_text(context, zHex, n * 2, sql_free);
> }
> }
Regards,
Timur
More information about the Tarantool-patches
mailing list