Re: [PATCH gwproxy v10 2/2] gwproxy: refactor code base to add experimental raw DNS backend

[Alviro Iskandar Setiawan <alviro.iskandar@gnuweeb.org>]

On Wed, Sep 10, 2025 at 5:44 PM Ahmad Gani wrote:
> +       cfg = &w->ctx->cfg;
> +       resolv = &w->dns_resolver;
> +       p1 = realloc(resolv->stack.arr, cfg->sess_map_cap * sizeof(*resolv->stack.arr));
> +       if (!p1)
> +               return;
> +       resolv->stack.arr = p1;
> +
> +       p2 = realloc(resolv->sess_map, cfg->sess_map_cap * sizeof(*resolv->sess_map));
> +       if (!p2)
> +               return;
> +       resolv->sess_map = p2;

This shrink operation looks very *dangerous* to me. Think, what
happens if "p2 = realloc()" fails?

You update "->stack.arr = p1;" but given "->stack.top >=
->sess_map_cap", now ->stack.top is pointing to an index beyond the
array capacity because you have just successfully shrunk the array
capacity down to ->sess_map_cap. Next time, a client with SOCKS5
hostname comes in, you call pop_txid(), and you will explode
   ->stack.arr[--resolv->stack.top] ** OVERFLOW **
because ->stack.top is not updated. That kind of pattern is only safe
for array expansion, not for this shrink op definitely. You're allowed
to over allocate the size, but not under allocate.

> +       memset(p2, 0, cfg->sess_map_cap * sizeof(*resolv->sess_map));
> +
> +       i = cfg->sess_map_cap;
> +       resolv->stack.top = i--;
> +       for (; i <= 0; i--)
> +               p1[i] = i;
> +
> +       resolv->sess_map_cap = cfg->sess_map_cap;

Meanwhile, ->stack.top *is* only updated when p2 is successfully
shrunk, what if *only the p1 succeeds*?

My hunch is that you may still need some basic training on C
programming. Today, I am still not confident with your overall series.
This feature is still far away from an acceptable state. For now, big
NAK from me on this series.

Anyway, this patch is doing too many things. It's not something you
should do in a single commit. Please split it into smaller, more
manageable pieces.

tq
-- 
Software Engineer & ITPM Officer
Alviro Iskandar Setiawan
+1 908 777 0074