diff mbox series

resolv: Fix a few unaligned accesses to fields in HEADER

Message ID 20231211125041.10418-1-ludwig.rydberg@gaisler.com
State Committed
Commit fc039ce8502d236f11074eb58468be24b4fa8cc7
Headers
Series resolv: Fix a few unaligned accesses to fields in HEADER |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent
redhat-pt-bot/TryBot-32bit success Build for i686
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_glibc_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_glibc_check--master-arm success Testing passed

Commit Message

Ludwig Rydberg Dec. 11, 2023, 12:50 p.m. UTC 
  After refactoring the alloca usage in 40c0add7d4 ("resolve: Remove
__res_context_query alloca usage") a few unaligned accesses to HEADER
fields surfaced. These unaligned accesses led to problems when running
the resolv test suite on sparc32-linux (leon) as many tests failed due to
SIGBUS crashes.

The issue(s) occured during T_QUERY_A_AND_AAAA queries as the second query
now can start on an unaligned address (previously it was explicitly aligned).

With this patch the unaligned accesses are now fixed by using the
UHEADER instead to ensure the fields are accessed with byte
loads/stores.

The patch has been verfied by running the resolv test suite on sparc32
and x86_64.

Signed-off-by: Ludwig Rydberg <ludwig.rydberg@gaisler.com>
Signed-off-by: Andreas Larsson <andreas@gaisler.com>
---
 resolv/res_mkquery.c      | 6 +++---
 resolv/res_queriesmatch.c | 6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)


base-commit: b3bee76c5f59498b9c189608f0a3132e2013fa1a

Comments

Florian Weimer Dec. 12, 2023, 8:28 a.m. UTC | #1 
* Ludwig Rydberg:

> After refactoring the alloca usage in 40c0add7d4 ("resolve: Remove
> __res_context_query alloca usage") a few unaligned accesses to HEADER
> fields surfaced. These unaligned accesses led to problems when running
> the resolv test suite on sparc32-linux (leon) as many tests failed due to
> SIGBUS crashes.
>
> The issue(s) occured during T_QUERY_A_AND_AAAA queries as the second query
> now can start on an unaligned address (previously it was explicitly aligned).
>
> With this patch the unaligned accesses are now fixed by using the
> UHEADER instead to ensure the fields are accessed with byte
> loads/stores.
>
> The patch has been verfied by running the resolv test suite on sparc32
> and x86_64.
>
> Signed-off-by: Ludwig Rydberg <ludwig.rydberg@gaisler.com>
> Signed-off-by: Andreas Larsson <andreas@gaisler.com>
> ---
>  resolv/res_mkquery.c      | 6 +++---
>  resolv/res_queriesmatch.c | 6 +++---
>  2 files changed, 6 insertions(+), 6 deletions(-)

Looks reasonable to me.  I will do some tests (although not on
strict-alignment targets, admittedly) and install it.

Reviewed-by: Florian Weimer <fweimer@redhat.com>

Thanks,
Florian
diff mbox series

Patch

diff --git a/resolv/res_mkquery.c b/resolv/res_mkquery.c
index e9316b3c3772..a46a92432b50 100644
--- a/resolv/res_mkquery.c
+++ b/resolv/res_mkquery.c
@@ -100,7 +100,7 @@  __res_context_mkquery (struct resolv_context *ctx, int op, const char *dname,
                        int class, int type, const unsigned char *data,
                        unsigned char *buf, int buflen)
 {
-  HEADER *hp;
+  UHEADER *hp;
   unsigned char *cp;
   int n;
   unsigned char *dnptrs[20], **dpp, **lastdnptr;
@@ -112,7 +112,7 @@  __res_context_mkquery (struct resolv_context *ctx, int op, const char *dname,
   if ((buf == NULL) || (buflen < HFIXEDSZ))
     return -1;
   memset (buf, 0, HFIXEDSZ);
-  hp = (HEADER *) buf;
+  hp = (UHEADER *) buf;
   /* We randomize the IDs every time.  The old code just incremented
      by one after the initial randomization which still predictable if
      the application does multiple requests.  */
@@ -250,7 +250,7 @@  __res_nopt (struct resolv_context *ctx,
             int n0, unsigned char *buf, int buflen, int anslen)
 {
   uint16_t flags = 0;
-  HEADER *hp = (HEADER *) buf;
+  UHEADER *hp = (UHEADER *) buf;
   unsigned char *cp = buf + n0;
   unsigned char *ep = buf + buflen;
 
diff --git a/resolv/res_queriesmatch.c b/resolv/res_queriesmatch.c
index ba1c1d0c0cf6..37db01a1b048 100644
--- a/resolv/res_queriesmatch.c
+++ b/resolv/res_queriesmatch.c
@@ -95,14 +95,14 @@  __libc_res_queriesmatch (const unsigned char *buf1, const unsigned char *eom1,
 
   /* Only header section present in replies to dynamic update
      packets.  */
-  if ((((HEADER *) buf1)->opcode == ns_o_update) &&
-      (((HEADER *) buf2)->opcode == ns_o_update))
+  if ((((UHEADER *) buf1)->opcode == ns_o_update) &&
+      (((UHEADER *) buf2)->opcode == ns_o_update))
     return 1;
 
   /* Note that we initially do not convert QDCOUNT to the host byte
      order.  We can compare it with the second buffer's QDCOUNT
      value without doing this.  */
-  int qdcount = ((HEADER *) buf1)->qdcount;
+  int qdcount = ((UHEADER *) buf1)->qdcount;
   if (qdcount != ((UHEADER *) buf2)->qdcount)
     return 0;