Using a structure for a tdb key can lead to insideous, hard

to find bugs. On 64 bit IRIX, structure packing means that
a
struct {
SMB_DEV_T dev /* 4 bytes */
SMB_INO_T ino /* 8 bytes */
}

has 4 bytes of padding between the two members. If you
don't null the memory before using it as a tdb key,
you randomly can't find keys depending on what is in
the padding. This caused me immense pain and was hard
to track down.... :-)

Jeremy.

diff --git a/source/locking/brlock.c b/source/locking/brlock.c
index 78a9174141d74f113557cf53941a18b35f87f03d..933fc142e904a3c83f9995301cca32a9b14f5679 100644 (file)
--- a/source/locking/brlock.c
+++ b/source/locking/brlock.c
@@ -63,6 +63,23 @@ struct lock_key {
 
 static TDB_CONTEXT *tdb;
 
+/****************************************************************************
+ Create a locking key - ensuring zero filled for pad purposes.
+****************************************************************************/
+
+static TDB_DATA locking_key(SMB_DEV_T dev, SMB_INO_T inode)
+{
+        static struct lock_key key;
+        TDB_DATA kbuf;
+
+        memset(&key, '\0', sizeof(key));
+        key.device = dev;
+        key.inode = inode;
+        kbuf.dptr = (char *)&key;
+        kbuf.dsize = sizeof(key);
+        return kbuf;
+}
+
 /****************************************************************************
  See if two locking contexts are equal.
 ****************************************************************************/
@@ -163,15 +180,11 @@ BOOL brl_lock(SMB_DEV_T dev, SMB_INO_T ino, int fnum,
              br_off start, br_off size, 
              enum brl_type lock_type)
 {
-       struct lock_key key;
        TDB_DATA kbuf, dbuf;
        int count, i;
        struct lock_struct lock, *locks;
 
-       key.device = dev;
-       key.inode = ino;
-       kbuf.dptr = (char *)&key;
-       kbuf.dsize = sizeof(key);
+       kbuf = locking_key(dev,ino);
 
        dbuf.dptr = NULL;
 
@@ -222,16 +235,12 @@ BOOL brl_unlock(SMB_DEV_T dev, SMB_INO_T ino, int fnum,
                uint16 smbpid, pid_t pid, uint16 tid,
                br_off start, br_off size)
 {
-       struct lock_key key;
        TDB_DATA kbuf, dbuf;
        int count, i;
        struct lock_struct *locks;
        struct lock_context context;
 
-       key.device = dev;
-       key.inode = ino;
-       kbuf.dptr = (char *)&key;
-       kbuf.dsize = sizeof(key);
+       kbuf = locking_key(dev,ino);
 
        dbuf.dptr = NULL;
 
@@ -305,15 +314,11 @@ BOOL brl_locktest(SMB_DEV_T dev, SMB_INO_T ino, int fnum,
                  br_off start, br_off size, 
                  enum brl_type lock_type)
 {
-       struct lock_key key;
        TDB_DATA kbuf, dbuf;
        int count, i;
        struct lock_struct lock, *locks;
 
-       key.device = dev;
-       key.inode = ino;
-       kbuf.dptr = (char *)&key;
-       kbuf.dsize = sizeof(key);
+       kbuf = locking_key(dev,ino);
 
        dbuf.dptr = NULL;
 
@@ -356,15 +361,11 @@ BOOL brl_locktest(SMB_DEV_T dev, SMB_INO_T ino, int fnum,
 
 void brl_close(SMB_DEV_T dev, SMB_INO_T ino, pid_t pid, int tid, int fnum)
 {
-       struct lock_key key;
        TDB_DATA kbuf, dbuf;
        int count, i, dcount=0;
        struct lock_struct *locks;
 
-       key.device = dev;
-       key.inode = ino;
-       kbuf.dptr = (char *)&key;
-       kbuf.dsize = sizeof(key);
+       kbuf = locking_key(dev,ino);
 
        dbuf.dptr = NULL;
 

diff --git a/source/locking/locking.c b/source/locking/locking.c
index 07411e89191e012fa1546aba3a3c8311211330a0..302b5b56c98b3c6b8f3586f66f3906bee2d7401d 100644 (file)
--- a/source/locking/locking.c
+++ b/source/locking/locking.c
@@ -255,6 +255,8 @@ static TDB_DATA locking_key(SMB_DEV_T dev, SMB_INO_T inode)
 {
        static struct locking_key key;
        TDB_DATA kbuf;
+
+       memset(&key, '\0', sizeof(key));
        key.dev = dev;
        key.inode = inode;
        kbuf.dptr = (char *)&key;

diff --git a/source/locking/posix.c b/source/locking/posix.c
index 0ab46f9ca44682a01cea12769e661a31994cb93d..d1edb1ef570e8d27cda491da37c21cd728f849ec 100644 (file)
--- a/source/locking/posix.c
+++ b/source/locking/posix.c
@@ -73,6 +73,8 @@ static TDB_DATA locking_key(SMB_DEV_T dev, SMB_INO_T inode)
 {
        static struct posix_lock_key key;
        TDB_DATA kbuf;
+
+       memset(&key, '\0', sizeof(key));
        key.device = dev;
        key.inode = inode;
        kbuf.dptr = (char *)&key;