Interesting issue with NDR_NOALIGN and nbt encoding.

[Jeremy Allison]

On Wed, May 23, 2012 at 09:25:08PM -0700, Jeremy Allison wrote:
> Sorry for this being so long, but the upshot of all
> this is I think that the flags
> 
> LIBNDR_FLAG_ALIGN2|LIBNDR_FLAG_ALIGN4|LIBNDR_FLAG_ALIGN8
> 
> which are collectively defined as LIBNDR_ALIGN_FLAGS,
> should be mutually exclusive with LIBNDR_FLAG_NOALIGN,
> in that when you set LIBNDR_FLAG_NOALIGN, the bits
> of LIBNDR_ALIGN_FLAGS should be removed, and when you
> set any of the LIBNDR_ALIGN_FLAGS bits, LIBNDR_FLAG_NOALIGN
> should be removed.
> 
> If we do this correctly I think it then allows the
> nbt_netlogon_query_for_pdc struct to be correctly
> marshalled/unmarshalled by the gen_ndr generated
> code. I'm wondering if it also may remove the
> need for some of the hand-generation of this code
> that got put into master ?
> 
> So attached is a patch to do just that. I'm testing
> it currently in master, but I'd really like to get
> feedback if I've missed something as to why it is
> allowed to set any of LIBNDR_FLAG_ALIGN2|LIBNDR_FLAG_ALIGN4|LIBNDR_FLAG_ALIGN8
> concurrently with LIBNDR_FLAG_NOALIGN.
> 
> This really doesn't seem right to me, and if it IS
> explicitly part of the design, I'd like to know what
> that design is.

More elegent patch that does the same thing whilst
modifying less code.

Jeremy.
-------------- next part --------------
diff --git a/librpc/ndr/libndr.h b/librpc/ndr/libndr.h
index 37a3145..9e51fd9 100644
--- a/librpc/ndr/libndr.h
+++ b/librpc/ndr/libndr.h
@@ -135,7 +135,7 @@ struct ndr_print {
 #define LIBNDR_FLAG_ALIGN4       (1<<23)
 #define LIBNDR_FLAG_ALIGN8       (1<<24)
 
-#define LIBNDR_ALIGN_FLAGS (LIBNDR_FLAG_ALIGN2|LIBNDR_FLAG_ALIGN4|LIBNDR_FLAG_ALIGN8)
+#define LIBNDR_ALIGN_FLAGS (LIBNDR_FLAG_NOALIGN|LIBNDR_FLAG_ALIGN2|LIBNDR_FLAG_ALIGN4|LIBNDR_FLAG_ALIGN8)
 
 #define LIBNDR_PRINT_ARRAY_HEX   (1<<25)
 #define LIBNDR_PRINT_SET_VALUES  (1<<26)
diff --git a/librpc/ndr/ndr.c b/librpc/ndr/ndr.c
index 2279d1c..3b83b65 100644
--- a/librpc/ndr/ndr.c
+++ b/librpc/ndr/ndr.c
@@ -383,6 +383,10 @@ _PUBLIC_ void ndr_set_flags(uint32_t *pflags, uint32_t new_flags)
 	}
 	if (new_flags & LIBNDR_ALIGN_FLAGS) {
 		(*pflags) &= ~LIBNDR_FLAG_REMAINING;
+		/* Ensure we only have the align
+		   flag set in the new_flags, remove
+		   any old align flag. */
+		(*pflags) &= ~LIBNDR_ALIGN_FLAGS;
 	}
 	if (new_flags & LIBNDR_FLAG_NO_RELATIVE_REVERSE) {
 		(*pflags) &= ~LIBNDR_FLAG_RELATIVE_REVERSE;