]> git.michaelhowe.org Git - packages/o/openafs.git/commitdiff
afs: Zero uninitialized uio structs
authorAndrew Deason <adeason@sinenomine.net>
Fri, 30 Jan 2015 19:29:57 +0000 (13:29 -0600)
committerStephan Wiesand <stephan.wiesand@desy.de>
Fri, 13 Feb 2015 10:06:30 +0000 (05:06 -0500)
In several places in the code, we allocate a 'struct uio' on the
stack, or allocate one from non-zeroed memory. In most of these
places, we initialize the structure by assigning individual fields to
certain values. However, this leaves any remaining fields assigned to
random garbage, if there are any additional fields in the struct uio
that we don't know about.

One such platform is Solaris, which has a field called uio_extflg,
which exists in Solaris 11, Solaris 10, and possibly further back.
One of the flags defined for this field in Solaris 11 is UIO_XUIO,
which indicates that the structure is actually an xuio_t, which is
larger than a normal uio_t and contains additional fields. So when we
allocate a uio on the stack without initializing it, it can randomly
appear to be an xuio_t, depending on what garbage was on the stack at
the time. An xuio_t is a kind of extensible structure, which is used
for things like async I/O or DMA, that kind of thing.

One of the places we make use of such a uio_t is in afs_ustrategy,
which we go through for cache reads and writes on most Unix platforms
(but not Linux). When handling a read (reading from the disk cache
into a mapped page), a copy of our stack-allocated uio eventually gets
passed to VOP_READ. So VOP_READ for the cache filesystem will randomly
interpret our uio_t as an xuio_t.

In many scenarios, this (amazingly) does not cause any problems, since
generally, Solaris code will not notice if something is flagged as an
xuio_t, unless it is specifically written to handle specific xuio_t
types. ZFS is one of the apparent few filesystem implementations that
can handle xuio_t's, and will detect and specially handle a
UIOTYPE_ZEROCOPY xuio_t differently than a regular uio_t.

If ZFS gets a UIOTYPE_ZEROCOPY xuio_t, it appears to ignore the uio
buffers passed in, and supplies its own buffers from its cache. This
means that our VOP_READ request will return success, and act like it
serviced the read just fine. However, the actual buffer that we passed
in will remain untouched, and so we will return the page to the VFS
filled with garbage data.

The way this typically manifests is that seemingly random pages will
contain random data. This seems to happen very rarely, though it may
not always be obvious what is going on when this occurs.

It is also worth noting that the above description on Solaris only
happens with Solaris 11 and newer, and only with a ZFS disk cache.
Anything older than Solaris 11 does not have the xuio_t framework
(though other uio_extflg values can cause performance degradations),
and all known non-ZFS local disk filesystems do not interpret special
xuio_t structures (networked filesystems might have xuio_t handling,
but they shouldn't be used for a cache).

Bugs similar to this may also exist on other Unix clients, but at
least this specific scenario should not occur on Linux (since we don't
use afs_ustrategy), and newer Darwin (since we get a uio allocated for
us).

To fix this, zero out the entire uio structure before we use it, for
all instances where we allocate a uio from the stack or from
non-zeroed memory. Also zero out the accompanying iovec in many
places, just to be safe. Some of these may not actually need to be
zeroed (since we do actually initialize the whole thing, or a platform
doesn't have any additional unknown uio fields), but it seems
worthwhile to err on the side of caution.

Thanks to Oracle for their assistance on this issue, and thanks to the
organization experiencing this issue for their patience and
persistence.

1.6 note: This differs noticeably from the master commit in two
places:

 - src/afs/NBSD/osi_vnodeops.c: On master there is no stack-allocated
   uio struct here.

 - src/afs/VNOPS/afs_vnop_write.c and afs_vnop_read.c: On master,
   these code paths are structured quite differently, and are handled
   in afs_osi_uio.c instead.

Reviewed-on: http://gerrit.openafs.org/11705
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Perry Ruiter <pruiter@sinenomine.net>
Reviewed-by: Daria Brashear <shadow@your-file-system.com>
(cherry picked from commit 5ef1de5eddccce0e7b135bb9ed4decaa62fc19ce)

Change-Id: I8dbf60637774dff81ff839ccd78f58b3b1e85c5b
Reviewed-on: http://gerrit.openafs.org/11713
Reviewed-by: Perry Ruiter <pruiter@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
14 files changed:
src/afs/AIX/osi_misc.c
src/afs/AIX/osi_vnodeops.c
src/afs/DARWIN/osi_vnodeops.c
src/afs/FBSD/osi_vnodeops.c
src/afs/HPUX/osi_vnodeops.c
src/afs/LINUX/osi_file.c
src/afs/LINUX/osi_vnodeops.c
src/afs/LINUX24/osi_file.c
src/afs/LINUX24/osi_vnodeops.c
src/afs/NBSD/osi_vnodeops.c
src/afs/OBSD/osi_vnodeops.c
src/afs/VNOPS/afs_vnop_read.c
src/afs/VNOPS/afs_vnop_strategy.c
src/afs/VNOPS/afs_vnop_write.c

index 4994ac1b78633d18529f776de992e9f0519a43b5..ed145c2d46c3f6e8ffc09ac68faa5a3677607a80 100644 (file)
@@ -97,6 +97,9 @@ gop_rdwr(rw, vp, base, len, offset, segflg, unit, aresid)
     struct iovec uiovector;
     int code;
 
+    memset(&uio_struct, 0, sizeof(uio_struct));
+    memset(&uiovector, 0, sizeof(uiovector));
+
     AFS_STATCNT(gop_rdwr);
     /* Set up the uio structure */
     uiovector.iov_base = (caddr_t) base;
index b86347e163a11410386a160fd8be1f6fd4952efb..7fb5556fd94b7db23510086fd89ce7279c253e6c 100644 (file)
@@ -643,6 +643,9 @@ afs_gn_fclear(struct vnode *vp,
     static int fclear_init = 0;
     struct vcache *avc = VTOAFS(vp);
 
+    memset(&uio, 0, sizeof(uio));
+    memset(&iov, 0, sizeof(iov));
+
     AFS_STATCNT(afs_gn_fclear);
     if (!fclear_init) {
        memset(zero_buffer, 0, PAGESIZE);
@@ -919,6 +922,9 @@ afs_vm_rdwr(struct vnode *vp,
            struct iovec tvec[16];      /* Should have access to #define */
            afs_int32 tsize;
 
+           memset(&tuio, 0, sizeof(tuio));
+           memset(&tvec, 0, sizeof(tvec));
+
            mixed = 1;
            finalOffset = xfrOffset + xfrSize;
            tsize = (afs_size_t) (xfrOffset + xfrSize - afs_vmMappingEnd);
@@ -1086,6 +1092,9 @@ afs_vm_rdwr(struct vnode *vp,
            struct uio tuio;
            struct iovec tvec[16];      /* Should have access to #define */
 
+           memset(&tuio, 0, sizeof(tuio));
+           memset(&tvec, 0, sizeof(tvec));
+
            /* Purge dirty chunks of file if there are too many dirty chunks.
             * Inside the write loop, we only do this at a chunk boundary.
             * Clean up partial chunk if necessary at end of loop.
index 54c07ad6c6e4d291849fa05ef45795c43b6d738d..d2a3d423490081807f9fe547b3565e38666e0f7e 100644 (file)
@@ -837,17 +837,20 @@ afs_vop_pagein(ap)
     int flags = ap->a_flags;
     struct ucred *cred;
     vm_offset_t ioaddr;
+    int code;
+    struct vcache *tvc = VTOAFS(vp);
+    int nocommit = flags & UPL_NOCOMMIT;
 #ifdef AFS_DARWIN80_ENV
     struct uio *uio;
 #else
     struct uio auio;
     struct iovec aiov;
     struct uio *uio = &auio;
+
+    memset(&auio, 0, sizeof(auio));
+    memset(&aiov, 0, sizeof(aiov));
 #endif
-    int nocommit = flags & UPL_NOCOMMIT;
 
-    int code;
-    struct vcache *tvc = VTOAFS(vp);
 #ifndef AFS_DARWIN80_ENV
     if (UBCINVALID(vp)) {
 #if DIAGNOSTIC
@@ -982,18 +985,21 @@ afs_vop_pageout(ap)
     int flags = ap->a_flags;
     struct ucred *cred;
     vm_offset_t ioaddr;
+    int nocommit = flags & UPL_NOCOMMIT;
+    int iosize;
+    int code;
+    struct vcache *tvc = VTOAFS(vp);
 #ifdef AFS_DARWIN80_ENV
     struct uio *uio;
 #else
     struct uio auio;
     struct iovec aiov;
     struct uio *uio = &auio;
+
+    memset(&auio, 0, sizeof(auio));
+    memset(&aiov, 0, sizeof(aiov));
 #endif
-    int nocommit = flags & UPL_NOCOMMIT;
-    int iosize;
 
-    int code;
-    struct vcache *tvc = VTOAFS(vp);
 #ifndef AFS_DARWIN80_ENV
     if (UBCINVALID(vp)) {
 #if DIAGNOSTIC
index 7a521524e573a961743c06d4eda3ecc3e89e5361..d407033fdb23a8ffb23da49cf70234449ddeb881 100644 (file)
@@ -801,6 +801,9 @@ afs_vop_getpages(struct vop_getpages_args *ap)
     struct vnode *vp;
     struct vcache *avc;
 
+    memset(&uio, 0, sizeof(uio));
+    memset(&iov, 0, sizeof(iov));
+
     vp = ap->a_vp;
     avc = VTOAFS(vp);
     if ((object = vp->v_object) == NULL) {
@@ -993,6 +996,9 @@ afs_vop_putpages(struct vop_putpages_args *ap)
     struct vnode *vp;
     struct vcache *avc;
 
+    memset(&uio, 0, sizeof(uio));
+    memset(&iov, 0, sizeof(iov));
+
     vp = ap->a_vp;
     avc = VTOAFS(vp);
     /* Perhaps these two checks should just be KASSERTs instead... */
index dab15dbfecc2dd950608f3dff3df8b429e270ca6..efba180533b099e8396035ef923c2214ffc19d69 100644 (file)
@@ -222,6 +222,9 @@ afs_bread(vp, lbn, bpp)
     struct iovec iov;
     struct uio uio;
 
+    memset(&uio, 0, sizeof(uio));
+    memset(&iov, 0, sizeof(iov));
+
     AFS_STATCNT(afs_bread);
     fsbsize = vp->v_vfsp->vfs_bsize;
     offset = lbn * fsbsize;
@@ -2085,6 +2088,9 @@ afs_readdir(vp, uiop, cred)
     dir_off_t offset;
     uint64_t tmp_offset;
 
+    memset(&auio, 0, sizeof(auio));
+    memset(&aiov, 0, sizeof(aiov));
+
     count = uiop->uio_resid;
     /* Allocate temporary space for format conversion */
     ibuf = kmem_alloc(2 * count);      /* overkill - fix later */
@@ -2151,6 +2157,9 @@ afs_readdir3(vp, uiop, cred)
     int count, outcount;
     dir_off_t offset;
 
+    memset(&auio, 0, sizeof(auio));
+    memset(&aiov, 0, sizeof(aiov));
+
     count = uiop->uio_resid;
     /* Allocate temporary space for format conversion */
     ibuf = kmem_alloc(2 * count);      /* overkill - fix later */
@@ -2521,6 +2530,9 @@ afs_hp_strategy(bp)
     extern caddr_t hdl_kmap_bp();
     struct kthread *t = u.u_kthreadp;
 
+    memset(&tuio, 0, sizeof(tuio));
+    memset(&tiovec, 0, sizeof(tiovec));
+
     AFS_STATCNT(afs_hp_strategy);
     /*
      * hdl_kmap_bp() saves "b_bcount" and restores it in hdl_remap_bp() after
index b83f736f51044eb3c2b60a2fb97d2574285a9c36..6261c93e71cabc2c631a6166944b783b2c3a8dbb 100644 (file)
@@ -209,6 +209,9 @@ afs_osi_Read(struct osi_file *afile, int offset, void *aptr,
     struct iovec iov;
     afs_int32 code;
 
+    memset(&auio, 0, sizeof(auio));
+    memset(&iov, 0, sizeof(iov));
+
     AFS_STATCNT(osi_Read);
 
     /*
@@ -250,6 +253,9 @@ afs_osi_Write(struct osi_file *afile, afs_int32 offset, void *aptr,
     struct iovec iov;
     afs_int32 code;
 
+    memset(&auio, 0, sizeof(auio));
+    memset(&iov, 0, sizeof(iov));
+
     AFS_STATCNT(osi_Write);
 
     if (!afile) {
index ea274f6d0bb31d77f8bc1a9637cbccc1a87b1338..216b4e2926f4b98fba50ec1e426b3120192f875b 100644 (file)
@@ -1830,6 +1830,9 @@ afs_linux_ireadlink(struct inode *ip, char *target, int maxlen, uio_seg_t seg)
     struct uio tuio;
     struct iovec iov;
 
+    memset(&tuio, 0, sizeof(tuio));
+    memset(&iov, 0, sizeof(iov));
+
     setup_uio(&tuio, &iov, target, (afs_offs_t) 0, maxlen, UIO_READ, seg);
     code = afs_readlink(VTOAFS(ip), &tuio, credp);
     crfree(credp);
@@ -2582,6 +2585,9 @@ afs_linux_page_writeback(struct inode *ip, struct page *pp,
     struct iovec iovec;
     int f_flags = 0;
 
+    memset(&tuio, 0, sizeof(tuio));
+    memset(&iovec, 0, sizeof(iovec));
+
     buffer = kmap(pp) + offset;
     base = page_offset(pp) + offset;
 
index 35bd031d00bd96afbb23ad0cd1dd5e37fc96759c..612db686d978872f5641826f7dd2cfa591b1a706 100644 (file)
@@ -190,6 +190,9 @@ afs_osi_Read(struct osi_file *afile, int offset, void *aptr,
     struct iovec iov;
     afs_int32 code;
 
+    memset(&auio, 0, sizeof(auio));
+    memset(&iov, 0, sizeof(iov));
+
     AFS_STATCNT(osi_Read);
 
     /*
@@ -231,6 +234,9 @@ afs_osi_Write(struct osi_file *afile, afs_int32 offset, void *aptr,
     struct iovec iov;
     afs_int32 code;
 
+    memset(&auio, 0, sizeof(auio));
+    memset(&iov, 0, sizeof(iov));
+
     AFS_STATCNT(osi_Write);
 
     if (!afile) {
index 9ac0b1ca362aae896e0cb9083937e43c3c4a7158..c64c427324f25ea9c46a4f14b236bcdbd1859b43 100644 (file)
@@ -1544,6 +1544,9 @@ afs_linux_ireadlink(struct inode *ip, char *target, int maxlen, uio_seg_t seg)
     struct uio tuio;
     struct iovec iov;
 
+    memset(&tuio, 0, sizeof(tuio));
+    memset(&iov, 0, sizeof(iov));
+
     setup_uio(&tuio, &iov, target, (afs_offs_t) 0, maxlen, UIO_READ, seg);
     code = afs_readlink(VTOAFS(ip), &tuio, credp);
     crfree(credp);
@@ -1812,6 +1815,9 @@ afs_linux_writepage_sync(struct inode *ip, struct page *pp,
     struct iovec iovec;
     int f_flags = 0;
 
+    memset(&tuio, 0, sizeof(tuio));
+    memset(&iovec, 0, sizeof(iovec));
+
     buffer = kmap(pp) + offset;
     base = (((loff_t) pp->index) << PAGE_CACHE_SHIFT)  + offset;
 
@@ -1921,6 +1927,9 @@ afs_linux_updatepage(struct file *fp, struct page *pp, unsigned long offset,
     struct uio tuio;
     struct iovec iovec;
 
+    memset(&tuio, 0, sizeof(tuio));
+    memset(&iovec, 0, sizeof(iovec));
+
     set_bit(PG_locked, &pp->flags);
 
     credp = crref();
index 48e29f09fb2eb9176846cc064c9597f14ed4b7a0..a6286b1a069195b0b4a1e6c0ec1154e558c882e1 100644 (file)
@@ -1035,6 +1035,9 @@ afs_nbsd_strategy(void *v)
     long len = abp->b_bcount;
     int code;
 
+    memset(&tuio, 0, sizeof(tuio));
+    memset(&tiovec, 0, sizeof(tiovec));
+
     AFS_STATCNT(afs_strategy);
 
     tuio.afsio_iov = tiovec;
index 141c88172cdabf11bd665e155140ca2d3985cd5f..28a0e73d58f8d5297250c8c561df279d9e803826 100644 (file)
@@ -1033,6 +1033,9 @@ afs_obsd_strategy(void *v)
     long len = abp->b_bcount;
     int code;
 
+    memset(&tuio, 0, sizeof(tuio));
+    memset(&tiovec, 0, sizeof(tiovec));
+
     AFS_STATCNT(afs_strategy);
 
     tuio.afsio_iov = tiovec;
index 3bad235a1177f97ba03ba9eee530d32781f6973b..9ca292945fadb7e86ca48213bb5e7b04e180fd21 100644 (file)
@@ -54,15 +54,16 @@ afs_MemRead(struct vcache *avc, struct uio *auio,
     afs_int32 trimlen;
     struct dcache *tdc = 0;
     afs_int32 error, trybusy = 1;
+    afs_int32 code;
+    struct vrequest *treq = NULL;
 #ifdef AFS_DARWIN80_ENV
     uio_t tuiop = NULL;
 #else
     struct uio tuio;
     struct uio *tuiop = &tuio;
     struct iovec *tvec;
+    memset(&tuio, 0, sizeof(tuio));
 #endif
-    afs_int32 code;
-    struct vrequest *treq = NULL;
 
     AFS_STATCNT(afs_MemRead);
     if (avc->vc_error)
@@ -93,6 +94,7 @@ afs_MemRead(struct vcache *avc, struct uio *auio,
 
 #ifndef AFS_DARWIN80_ENV
     tvec = (struct iovec *)osi_AllocSmallSpace(sizeof(struct iovec));
+    memset(tvec, 0, sizeof(struct iovec));
 #endif
     totalLength = AFS_UIO_RESID(auio);
     filePos = AFS_UIO_OFFSET(auio);
@@ -513,17 +515,18 @@ afs_UFSRead(struct vcache *avc, struct uio *auio,
     afs_int32 trimlen;
     struct dcache *tdc = 0;
     afs_int32 error;
+    struct osi_file *tfile;
+    afs_int32 code;
+    int trybusy = 1;
+    struct vrequest *treq = NULL;
 #ifdef AFS_DARWIN80_ENV
     uio_t tuiop=NULL;
 #else
     struct uio tuio;
     struct uio *tuiop = &tuio;
     struct iovec *tvec;
+    memset(&tuio, 0, sizeof(tuio));
 #endif
-    struct osi_file *tfile;
-    afs_int32 code;
-    int trybusy = 1;
-    struct vrequest *treq = NULL;
 
     AFS_STATCNT(afs_UFSRead);
     if (avc && avc->vc_error)
@@ -562,6 +565,7 @@ afs_UFSRead(struct vcache *avc, struct uio *auio,
 
 #ifndef AFS_DARWIN80_ENV
     tvec = (struct iovec *)osi_AllocSmallSpace(sizeof(struct iovec));
+    memset(tvec, 0, sizeof(struct iovec));
 #endif
     totalLength = AFS_UIO_RESID(auio);
     filePos = AFS_UIO_OFFSET(auio);
index eed0e136a57e9c29d2ce9b13716d9f4640de2bdb..599a836b25e7d3918e0dd71afdbcd80d73332972 100644 (file)
@@ -46,6 +46,9 @@ int afs_ustrategy(struct buf *abp)
     afs_ucred_t *credp = u.u_cred;
 #endif
 
+    memset(&tuio, 0, sizeof(tuio));
+    memset(&tiovec, 0, sizeof(tiovec));
+
     AFS_STATCNT(afs_ustrategy);
 #ifdef AFS_AIX41_ENV
     /*
index 068dbac5dff7da1b65dc9faccd6bb31af9f6eb6e..d11cdbcf8911dce188fe9a092717b7ffd53e0ee9 100644 (file)
@@ -113,15 +113,16 @@ afs_MemWrite(struct vcache *avc, struct uio *auio, int aio,
 #if defined(AFS_FBSD_ENV) || defined(AFS_DFBSD_ENV)
     struct vnode *vp = AFSTOV(avc);
 #endif
+    afs_int32 code;
+    struct vrequest *treq = NULL;
 #ifdef AFS_DARWIN80_ENV
     uio_t tuiop = NULL;
 #else
     struct uio tuio;
     struct uio *tuiop = &tuio;
     struct iovec *tvec;                /* again, should have define */
+    memset(&tuio, 0, sizeof(tuio));
 #endif
-    afs_int32 code;
-    struct vrequest *treq = NULL;
 
     AFS_STATCNT(afs_MemWrite);
     if (avc->vc_error)
@@ -201,6 +202,7 @@ afs_MemWrite(struct vcache *avc, struct uio *auio, int aio,
     avc->f.states |= CDirty;
 #ifndef AFS_DARWIN80_ENV
     tvec = (struct iovec *)osi_AllocSmallSpace(sizeof(struct iovec));
+    memset(tvec, 0, sizeof(struct iovec));
 #endif
     while (totalLength > 0) {
        tdc = afs_ObtainDCacheForWriting(avc, filePos, totalLength, treq,
@@ -332,16 +334,17 @@ afs_UFSWrite(struct vcache *avc, struct uio *auio, int aio,
 #if defined(AFS_FBSD_ENV) || defined(AFS_DFBSD_ENV)
     struct vnode *vp = AFSTOV(avc);
 #endif
+    struct osi_file *tfile;
+    afs_int32 code;
+    struct vrequest *treq = NULL;
 #ifdef AFS_DARWIN80_ENV
     uio_t tuiop = NULL;
 #else
     struct uio tuio;
     struct uio *tuiop = &tuio;
     struct iovec *tvec;                /* again, should have define */
+    memset(&tuio, 0, sizeof(tuio));
 #endif
-    struct osi_file *tfile;
-    afs_int32 code;
-    struct vrequest *treq = NULL;
 
     AFS_STATCNT(afs_UFSWrite);
     if (avc->vc_error)
@@ -424,6 +427,7 @@ afs_UFSWrite(struct vcache *avc, struct uio *auio, int aio,
     avc->f.states |= CDirty;
 #ifndef AFS_DARWIN80_ENV
     tvec = (struct iovec *)osi_AllocSmallSpace(sizeof(struct iovec));
+    memset(tvec, 0, sizeof(struct iovec));
 #endif
     while (totalLength > 0) {
        tdc = afs_ObtainDCacheForWriting(avc, filePos, totalLength, treq,