From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server-vie001.gnuweeb.org X-Spam-Level: X-Spam-Status: No, score=-1.2 required=5.0 tests=ALL_TRUSTED,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,URIBL_DBL_BLOCKED_OPENDNS, URIBL_ZEN_BLOCKED_OPENDNS autolearn=ham autolearn_force=no version=3.4.6 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gnuweeb.org; s=new2025; t=1755156501; bh=zX370mlBvjkSc+FWHs2QkL+sBmXv3k+5yu+SSogzcvw=; h=MIME-Version:References:In-Reply-To:From:Date:Message-ID:Subject: To:Cc:Content-Type:Content-Transfer-Encoding:Message-ID:Date:From: Reply-To:Subject:To:Cc:In-Reply-To:References:Resent-Date: Resent-From:Resent-To:Resent-Cc:User-Agent:Content-Type: Content-Transfer-Encoding; b=mGvRBz5DXNd3Uu/q/u1XNUW1Q2m8KW7WRnwn/BCNvlRFpfx8E0s67vac3gMl94kLr HuCB5yDcOueCOirS22GCDyuHD0yoUjTUHYPwtjRPYYlc2fuRILxDmGqFlzcvbxHraw 8mDvAZxnjGxgpvBjQkqNSVCy4jcIuF950P0XFkbi7ocBzIDYbjVAHBM1mtD0ODS4Xs RNMZXYa/bSK6vUgVL+0AOHygNI52t0qcsETmR8cBdBehbINJXqplhEKeLUj7dw6/JW lcfyUmmLsxopKK36ChiyuPW2PCGnrvNgHorI+GU1u1XBlGiiDeECNQpfSQIIE6aqo+ OEbskHhX3Cbhw== Received: from mail-pj1-f43.google.com (mail-pj1-f43.google.com [209.85.216.43]) by server-vie001.gnuweeb.org (Postfix) with ESMTPSA id A91423127F9D for ; Thu, 14 Aug 2025 07:28:21 +0000 (UTC) Received: by mail-pj1-f43.google.com with SMTP id 98e67ed59e1d1-32326e5f0bfso640264a91.3 for ; Thu, 14 Aug 2025 00:28:21 -0700 (PDT) X-Forwarded-Encrypted: i=1; AJvYcCXIcnn9tFzwhNym+jux4qiY61s3oZE1uzdLAmprcI72jB4gk5zu3Ha0Gf01WnVUxVuDjHz7@vger.gnuweeb.org X-Gm-Message-State: AOJu0YxyGq19U5NqK0TqzrXPulta+IWFckUm3nPMjCBmVbyzrlgFCXWm biA+SqGvAUW/U3MHJttnGGw9efkCinzT1pZ/6WO0n8Pzgnp297Bw3qBj1LoGxLwNwdqwIKSueo3 SXjJ9KTgtdpG5uPviN5ckVtQqDaN96Go= X-Google-Smtp-Source: AGHT+IH7wDqok5Re7ZatOYfa8SnCk6M8pGqTtRfKJzJEs3elFbqH1EckubEGMYbqhPBo8gVxiYAZdGUXpmRUKcGm6lI= X-Received: by 2002:a17:90b:2807:b0:2fe:85f0:e115 with SMTP id 98e67ed59e1d1-3232b4c6e5bmr2528729a91.26.1755156500046; Thu, 14 Aug 2025 00:28:20 -0700 (PDT) MIME-Version: 1.0 References: <20250814044658.252579-1-reyuki@gnuweeb.org> <20250814044658.252579-3-reyuki@gnuweeb.org> In-Reply-To: <20250814044658.252579-3-reyuki@gnuweeb.org> From: Alviro Iskandar Setiawan Date: Thu, 14 Aug 2025 14:28:08 +0700 X-Gmail-Original-Message-ID: X-Gm-Features: Ac12FXzESQBzyCOVYQxnZIq70Bu3V-iTUhM7AxCWgjjNn1PLdO8xvK3J-vUZlOQ Message-ID: Subject: Re: [PATCH gwproxy v5 2/2] dnsparser: Add dns parser code To: Ahmad Gani Cc: Ammar Faizi , "GNU/Weeb Mailing List" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable List-Id: On Thu, Aug 14, 2025 at 11:53=E2=80=AFAM Ahmad Gani wrote: > +static int serialize_answ(uint16_t txid, uint8_t *in, size_t in_len, gwd= ns_answ_data *out) > +{ > + gwdns_header_pkt *hdr; > + uint16_t raw_flags; > + size_t idx, i; > + void *ptr; > + int ret; > + > + idx =3D sizeof(*hdr); > + if (idx >=3D in_len) > + return -EAGAIN; > + > + hdr =3D (void *)in; > + if (memcmp(&txid, &hdr->id, sizeof(txid))) > + return -EINVAL; > + > + memcpy(&raw_flags, &hdr->flags, sizeof(raw_flags)); > + raw_flags =3D ntohs(raw_flags); > + /* QR MUST 1 =3D response from dns server */ > + if (!DNS_QR(raw_flags)) > + return -EINVAL; > + > + /* OPCODE MUST 0 =3D standard query */ > + if (DNS_OPCODE(raw_flags)) > + return -EINVAL; > + > + /* RCODE MUST 0 =3D No error */ > + if (DNS_RCODE(raw_flags)) > + return -EPROTO; > + > + // is it safe or recommended to alter the in buffer directly? > + hdr->ancount =3D ntohs(hdr->ancount); > + if (!hdr->ancount) > + return -ENODATA; > + > + /* > + * Check the sizes upfront. > + * > + * 1 bytes for variable-length > + * in[idx] for the length of first name > + * 1 bytes for null terminator > + * 2 bytes for qtype > + * 2 bytes for qclass > + */ > + if ((size_t)(1 + in[idx] + 1 + 2 + 2) >=3D in_len) > + return -EINVAL; > + > + ret =3D calculate_question_len(&in[idx], in_len - idx); > + if (ret <=3D 0) > + return -EINVAL; > + > + idx +=3D ret; > + if (idx >=3D in_len) > + return -EAGAIN; > + > + out->hdr.ancount =3D 0; > + ptr =3D malloc(hdr->ancount * sizeof(uint8_t *)); > + if (!ptr) > + return -ENOMEM; > + > + out->rr_answ =3D ptr; > + for (i =3D 0; i < hdr->ancount; i++) { > + uint16_t is_compressed, rdlength; > + gwdns_serialized_answ *item =3D malloc(sizeof(gwdns_seria= lized_answ)); > + if (!item) { > + ret =3D -ENOMEM; > + goto exit_free; > + } > + > + memcpy(&is_compressed, &in[idx], sizeof(is_compressed)); > + is_compressed =3D DNS_IS_COMPRESSED(ntohs(is_compressed))= ; > + assert(is_compressed); > + idx +=3D 2; // NAME > + if (idx >=3D in_len) { > + ret =3D -EAGAIN; > + free(item); > + goto exit_free; > + } > + > + memcpy(&item->rr_type, &in[idx], 2); > + item->rr_type =3D ntohs(item->rr_type); > + idx +=3D 2; // TYPE > + if (idx >=3D in_len) { > + ret =3D -EAGAIN; > + free(item); > + goto exit_free; > + } > + memcpy(&item->rr_class, &in[idx], 2); > + item->rr_class =3D ntohs(item->rr_class); > + idx +=3D 2; // CLASS > + if (idx >=3D in_len) { > + ret =3D -EAGAIN; > + free(item); > + goto exit_free; > + } > + memcpy(&item->ttl, &in[idx], 4); > + item->ttl =3D be32toh(item->ttl); > + idx +=3D 4; // TTL > + if (idx >=3D in_len) { > + ret =3D -EAGAIN; > + free(item); > + goto exit_free; > + } > + > + memcpy(&rdlength, &in[idx], sizeof(rdlength)); > + rdlength =3D ntohs(rdlength); > + if (item->rr_type !=3D TYPE_AAAA && item->rr_type !=3D TY= PE_A) { > + ret =3D -EINVAL; > + free(item); > + goto exit_free; > + } > + if (item->rr_type =3D=3D TYPE_AAAA && rdlength !=3D sizeo= f(struct in6_addr)) { > + ret =3D -EINVAL; > + free(item); > + goto exit_free; > + } > + if (item->rr_type =3D=3D TYPE_A && rdlength !=3D sizeof(s= truct in_addr)) { > + ret =3D -EINVAL; > + free(item); > + goto exit_free; > + } > + item->rdlength =3D rdlength; > + idx +=3D 2; > + if (idx >=3D in_len) { > + ret =3D -EAGAIN; > + free(item); > + goto exit_free; > + } > + > + /* > + * considering if condition above, > + * maybe we don't need a malloc and just allocate fixed s= ize > + * for rdata? however if this parser want to be expanded = for > + * other dns operation (e.g OPCODE_IQUERY, etc), rdata ma= ybe > + * contain more than sizeof in6_addr. > + */ > + ptr =3D malloc(rdlength); > + if (!ptr) { > + ret =3D -ENOMEM; > + free(item); > + goto exit_free; > + } > + > + memcpy(ptr, &in[idx], rdlength); > + idx +=3D rdlength; > + if (idx > in_len) { > + ret =3D -EINVAL; > + free(item); > + free(ptr); > + goto exit_free; > + } > + > + item->rdata =3D ptr; > + out->rr_answ[i] =3D item; > + out->hdr.ancount++; > + } > + > + return 0; > +exit_free: > + for (i =3D 0; i < out->hdr.ancount; i++) { > + free(out->rr_answ[i]->rdata); > + free(out->rr_answ[i]); > + } > + free(out->rr_answ); > + return ret; > +} I think free(item); can be deduplicated. It's repeated several times here. Consider using goto exit_free_item and add the appropriate label. Do the same to other places if you have the same pattern elsewhere.