anyone?

On 2022-03-03 19:57 +01, Florian Obser <[email protected]> wrote:
> parse_packet() is used by unbound(8) to parse response packets, not
> queries. There is no need to do all this work just to get access to
> the query id and flags. This is what unbound(8) is doing.
>
> OK?
>
> diff --git frontend.c frontend.c
> index 6316231f4bf..ac53fc01ef1 100644
> --- frontend.c
> +++ frontend.c
> @@ -100,12 +100,13 @@ struct pending_query {
>       struct sldns_buffer             *abuf;
>       struct regional                 *region;
>       struct query_info                qinfo;
> -     struct msg_parse                *qmsg;
>       struct edns_data                 edns;
>       struct event                     ev;            /* for tcp */
>       struct event                     resp_ev;       /* for tcp */
>       struct event                     tmo_ev;        /* for tcp */
>       uint64_t                         imsg_id;
> +     uint16_t                         id;
> +     uint16_t                         flags;
>       int                              fd;
>       int                              tcp;
>       int                              dns64_synthesize;
> @@ -536,8 +537,7 @@ frontend_dispatch_resolver(int fd, short event, void 
> *bula)
>                               break;
>                       }
>  
> -                     if (answer_header->bogus && !(pq->qmsg->flags &
> -                         BIT_CD)) {
> +                     if (answer_header->bogus && !(pq->flags & BIT_CD)) {
>                               error_answer(pq, LDNS_RCODE_SERVFAIL);
>                               send_answer(pq);
>                               break;
> @@ -716,15 +716,13 @@ udp_receive(int fd, short events, void *arg)
>       pq->qbuf = sldns_buffer_new(len);
>       pq->abuf = sldns_buffer_new(len); /* make sure we can send errors */
>       pq->region = regional_create();
> -     pq->qmsg = regional_alloc(pq->region, sizeof(*pq->qmsg));
>  
> -     if (!pq->qbuf || !pq->abuf || !pq->region || !pq->qmsg) {
> +     if (!pq->qbuf || !pq->abuf || !pq->region) {
>               log_warnx("out of memory");
>               free_pending_query(pq);
>               return;
>       }
>  
> -     memset(pq->qmsg, 0, sizeof(*pq->qmsg));
>       sldns_buffer_write(pq->qbuf, udpev->query, len);
>       sldns_buffer_flip(pq->qbuf);
>       handle_query(pq);
> @@ -734,7 +732,6 @@ void
>  handle_query(struct pending_query *pq)
>  {
>       struct query_imsg        query_imsg;
> -     struct query_info        skip;
>       struct bl_node           find;
>       int                      rcode;
>       char                    *str;
> @@ -750,16 +747,16 @@ handle_query(struct pending_query *pq)
>               free(str);
>       }
>  
> -     if (!query_info_parse(&pq->qinfo, pq->qbuf)) {
> -             log_warnx("query_info_parse failed");
> +     if (sldns_buffer_remaining(pq->qbuf) < LDNS_HEADER_SIZE) {
> +             log_warnx("bad query: too short, dropped");
>               goto drop;
>       }
>  
> -     sldns_buffer_rewind(pq->qbuf);
> +     pq->id = sldns_buffer_read_u16_at(pq->qbuf, 0);
> +     pq->flags = sldns_buffer_read_u16_at(pq->qbuf, 2);
>  
> -     if (parse_packet(pq->qbuf, pq->qmsg, pq->region) !=
> -         LDNS_RCODE_NOERROR) {
> -             log_warnx("parse_packet failed");
> +     if (!query_info_parse(&pq->qinfo, pq->qbuf)) {
> +             log_warnx("query_info_parse failed");
>               goto drop;
>       }
>  
> @@ -774,11 +771,6 @@ handle_query(struct pending_query *pq)
>               goto send_answer;
>       }
>  
> -     sldns_buffer_rewind(pq->qbuf);
> -     if (!query_info_parse(&skip, pq->qbuf)) {
> -             error_answer(pq, LDNS_RCODE_SERVFAIL);
> -             goto send_answer;
> -     }
>       rcode = parse_edns_from_query_pkt(pq->qbuf, &pq->edns, NULL, NULL,
>           pq->region);
>       if (rcode != LDNS_RCODE_NOERROR) {
> @@ -891,7 +883,7 @@ noerror_answer(struct pending_query *pq)
>       }
>  
>       sldns_buffer_clear(pq->abuf);
> -     if (reply_info_encode(&pq->qinfo, rinfo, pq->qmsg->id, rinfo->flags,
> +     if (reply_info_encode(&pq->qinfo, rinfo, htons(pq->id), rinfo->flags,
>           pq->abuf, 0, pq->region, pq->tcp ? UINT16_MAX : pq->edns.udp_size,
>           pq->edns.bits & EDNS_DO, MINIMIZE_ANSWER) == 0)
>               goto srvfail;
> @@ -986,7 +978,7 @@ synthesize_dns64_answer(struct pending_query *pq)
>  
>       sldns_buffer_clear(pq->abuf);
>  
> -     if (reply_info_encode(&pq->qinfo, synth_rinfo, pq->qmsg->id,
> +     if (reply_info_encode(&pq->qinfo, synth_rinfo, htons(pq->id),
>           synth_rinfo->flags, pq->abuf, 0, pq->region,
>           pq->tcp ? UINT16_MAX : pq->edns.udp_size,
>           pq->edns.bits & EDNS_DO, MINIMIZE_ANSWER) == 0)
> @@ -1006,7 +998,6 @@ void
>  resend_dns64_query(struct pending_query *opq) {
>       struct pending_query    *pq;
>       struct query_imsg        query_imsg;
> -     struct query_info        skip;
>       int                      rcode;
>       char                     dname[LDNS_MAX_DOMAINLEN + 1];
>  
> @@ -1028,9 +1019,8 @@ resend_dns64_query(struct pending_query *opq) {
>       pq->qbuf = sldns_buffer_new(sldns_buffer_capacity(opq->qbuf));
>       pq->abuf = sldns_buffer_new(sldns_buffer_capacity(opq->abuf));
>       pq->region = regional_create();
> -     pq->qmsg = regional_alloc(pq->region, sizeof(*pq->qmsg));
>  
> -     if (!pq->qbuf || !pq->abuf || !pq->region || !pq->qmsg) {
> +     if (!pq->qbuf || !pq->abuf || !pq->region) {
>               log_warnx("out of memory");
>               free_pending_query(pq);
>               free_pending_query(opq);
> @@ -1041,7 +1031,6 @@ resend_dns64_query(struct pending_query *opq) {
>       sldns_buffer_write(pq->qbuf, sldns_buffer_current(opq->qbuf),
>           sldns_buffer_remaining(opq->qbuf));
>       sldns_buffer_flip(pq->qbuf);
> -     memset(pq->qmsg, 0, sizeof(*pq->qmsg));
>  
>       if (pq->tcp) {
>               struct timeval   timeout = {TCP_TIMEOUT, 0};
> @@ -1054,24 +1043,19 @@ resend_dns64_query(struct pending_query *opq) {
>               evtimer_add(&pq->tmo_ev, &timeout);
>       }
>  
> -     if (!query_info_parse(&pq->qinfo, pq->qbuf)) {
> -             log_warnx("query_info_parse failed");
> +     if (sldns_buffer_remaining(pq->qbuf) < LDNS_HEADER_SIZE) {
> +             log_warnx("bad query: too short, dropped");
>               goto drop;
>       }
>  
> -     sldns_buffer_rewind(pq->qbuf);
> +     pq->id = sldns_buffer_read_u16_at(pq->qbuf, 0);
> +     pq->flags = sldns_buffer_read_u16_at(pq->qbuf, 2);
>  
> -     if (parse_packet(pq->qbuf, pq->qmsg, pq->region) !=
> -         LDNS_RCODE_NOERROR) {
> -             log_warnx("parse_packet failed");
> +     if (!query_info_parse(&pq->qinfo, pq->qbuf)) {
> +             log_warnx("query_info_parse failed");
>               goto drop;
>       }
>  
> -     sldns_buffer_rewind(pq->qbuf);
> -     if (!query_info_parse(&skip, pq->qbuf)) {
> -             error_answer(pq, LDNS_RCODE_SERVFAIL);
> -             goto send_answer;
> -     }
>       rcode = parse_edns_from_query_pkt(pq->qbuf, &pq->edns, NULL, NULL,
>           pq->region);
>       if (rcode != LDNS_RCODE_NOERROR) {
> @@ -1154,8 +1138,8 @@ void
>  error_answer(struct pending_query *pq, int rcode)
>  {
>       sldns_buffer_clear(pq->abuf);
> -     error_encode(pq->abuf, rcode, &pq->qinfo, pq->qmsg->id,
> -         pq->qmsg->flags, pq->edns.edns_present ? &pq->edns : NULL);
> +     error_encode(pq->abuf, rcode, &pq->qinfo, htons(pq->id), pq->flags,
> +         pq->edns.edns_present ? &pq->edns : NULL);
>  }
>  
>  int
> @@ -1669,15 +1653,12 @@ tcp_accept(int fd, short events, void *arg)
>       pq->tcp = 1;
>       pq->qbuf = sldns_buffer_new(DEFAULT_TCP_SIZE);
>       pq->region = regional_create();
> -     pq->qmsg = regional_alloc(pq->region, sizeof(*pq->qmsg));
>  
> -     if (!pq->qbuf || !pq->region || !pq->qmsg) {
> +     if (!pq->qbuf || !pq->region) {
>               free_pending_query(pq);
>               return;
>       }
>  
> -     memset(pq->qmsg, 0, sizeof(*pq->qmsg));
> -
>       event_set(&pq->ev, s, EV_READ | EV_PERSIST, tcp_request, pq);
>       event_add(&pq->ev, NULL);
>       event_set(&pq->resp_ev, s, EV_WRITE | EV_PERSIST, tcp_response, pq);
>
> -- 
>
> I'm not entirely sure you are real.
>

-- 
I'm not entirely sure you are real.

Reply via email to