On Sun, Nov 21, 2021 at 04:51:45PM +0100, Florian Obser wrote:
> On 2021-11-20 21:16 +01, Otto Moerbeek <o...@drijf.net> wrote:
> > On Sat, Nov 20, 2021 at 06:44:58PM +0100, Florian Obser wrote:
> >
> >> On 2021-11-20 18:41 +01, Florian Obser <flor...@openbsd.org> wrote:
> >> > On 2021-11-20 18:19 +01, Florian Obser <flor...@openbsd.org> wrote:
> >> >
> >> >> +/*
> >> >> + * Clear AD flag in the answer.
> >> >> + */
> >> >> +static void
> >> >> +clear_ad(struct asr_result *ar)
> >> >> +{
> >> >> + struct asr_dns_header *h;
> >> >> + uint16_t flags;
> >> >> +
> >> >> + h = (struct asr_dns_header *)ar->ar_data;
> >> >> + flags = ntohs(h->flags);
> >> >> + flags &= ~(AD_MASK);
> >> >> + h->flags = htons(flags);
> >> >> +}
> >> >> +
> >> >
> >> > btw. is it possible that this is not alligned correctly on sparc64?
> >> >
> >> > should be do something like (not even compile tested)
> >> >
> >> > static void
> >> > clear_ad(struct asr_result *ar)
> >> > {
> >> > struct asr_dns_header h;
> >> >
> >> > memmove(&h, ar->ar_data, sizeof(h));
> >> > h.flags = ntohs(h.flags);
> >> > h.flags &= ~(AD_MASK);
> >> > h.flags = htons(h.flags);
> >> > memmove(ar->ar_data, &h, sizeof(h));
> >> > }
> >> >
> >>
> >> memcpy obviously, I was distracted by the copious amount of memmove in
> >> asr code...
> >
> > It is not needed to copy the "whole" header just to change the flags.
> > You could just copy out, modify and copy back the flags field only.
> >
> > otoh, it's just 12 bytes, so no big deal.
>
> right. So I have tried my patch (without the memcpy dance) on sparc64
> over udp and tcp and I have also tracked this down in the code. This
> should be fine as is. ar->ar_data comes directly out of malloc
> (reallocarray) in ensure_ibuf() and the struct is defined thusly:
>
> struct asr_dns_header {
> uint16_t id;
> uint16_t flags;
> uint16_t qdcount;
> uint16_t ancount;
> uint16_t nscount;
> uint16_t arcount;
> };
>
So that is indeed safe as long as nobody starts allocating packet
buffers in different ways,
-Otto
