[PATCH] Fix undefined behavior in NDR

Andreas Schneider asn at samba.org
Thu Dec 6 08:54:44 UTC 2018


On Thursday, 6 December 2018 08:43:22 CET Andreas Schneider via samba-
technical wrote:
> On Wednesday, 5 December 2018 22:32:17 CET Stefan Metzmacher via samba-
> 
> technical wrote:
> > Hi Andreas,
> 
> Hi metze,
> 
> > Why do we need this if we have checks in ndr_push_bytes()?
> > 
> > Adding the above two checks in ndr_push_bytes() might also be very
> > costly. Isn't the first check enough? It should also use unlikely()
> 
> updated patchset attached ...

As the patch already landed in master, here is a follow up patch to fix it.


	Andreas

-- 
Andreas Schneider                      asn at samba.org
Samba Team                             www.samba.org
GPG-ID:     8DFF53E18F2ABC8D8F3C92237EE0FC4DCC014E3D
-------------- next part --------------
>From 0fc5de8c5b4d0af04f3ae1745ad6dec0a30a8213 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Thu, 6 Dec 2018 09:35:15 +0100
Subject: [PATCH] librpc:ndr: Give the optimizer hints for ndr_push_bytes()

Also remove the redundant check in ndr_push_DATA_BLOB.

Signed-off-by: Andreas Schneider <asn at samba.org>
---
 librpc/ndr/ndr_basic.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/librpc/ndr/ndr_basic.c b/librpc/ndr/ndr_basic.c
index b488c7c22d9..3a5189570c5 100644
--- a/librpc/ndr/ndr_basic.c
+++ b/librpc/ndr/ndr_basic.c
@@ -719,10 +719,10 @@ _PUBLIC_ enum ndr_err_code ndr_pull_trailer_align(struct ndr_pull *ndr, size_t s
 */
 _PUBLIC_ enum ndr_err_code ndr_push_bytes(struct ndr_push *ndr, const uint8_t *data, uint32_t n)
 {
-	if (n == 0) {
+	if (unlikely(n == 0)) {
 		return NDR_ERR_SUCCESS;
 	}
-	if (data == NULL) {
+	if (unlikely(data == NULL)) {
 		return NDR_ERR_INVALID_POINTER;
 	}
 	NDR_PUSH_NEED_BYTES(ndr, n);
@@ -1335,10 +1335,6 @@ _PUBLIC_ enum ndr_err_code ndr_push_DATA_BLOB(struct ndr_push *ndr, int ndr_flag
 	} else {
 		NDR_CHECK(ndr_push_uint3264(ndr, NDR_SCALARS, blob.length));
 	}
-	if (blob.length == 0) {
-		return NDR_ERR_SUCCESS;
-	}
-
 	NDR_CHECK(ndr_push_bytes(ndr, blob.data, blob.length));
 	return NDR_ERR_SUCCESS;
 }
-- 
2.19.2



More information about the samba-technical mailing list