xfs: reset buffer pointers before freeing them
authorDave Chinner <dchinner@redhat.com>
Thu, 21 Apr 2011 09:34:27 +0000 (09:34 +0000)
committerAlex Elder <aelder@sgi.com>
Thu, 19 May 2011 17:03:45 +0000 (12:03 -0500)
When we free a vmapped buffer, we need to ensure the vmap address
and length we free is the same as when it was allocated. In various
places in the log code we change the memory the buffer is pointing
to before issuing IO, but we never reset the buffer to point back to
it's original memory (or no memory, if that is the case for the
buffer).

As a result, when we free the buffer it points to memory that is
owned by something else and attempts to unmap and free it. Because
the range does not match any known mapped range, it can trigger
BUG_ON() traps in the vmap code, and potentially corrupt the vmap
area tracking.

Fix this by always resetting these buffers to their original state
before freeing them.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Alex Elder <aelder@sgi.com>
fs/xfs/linux-2.6/xfs_buf.c
fs/xfs/linux-2.6/xfs_buf.h
fs/xfs/xfs_log.c
fs/xfs/xfs_log_recover.c

index 098890357659b4b6fbf59c78b2d092b55e1c1d71..52b2b5da566e7ff801adda6b7c2f57147f39fc80 100644 (file)
@@ -708,6 +708,27 @@ xfs_buf_get_empty(
        return bp;
 }
 
+/*
+ * Return a buffer allocated as an empty buffer and associated to external
+ * memory via xfs_buf_associate_memory() back to it's empty state.
+ */
+void
+xfs_buf_set_empty(
+       struct xfs_buf          *bp,
+       size_t                  len)
+{
+       if (bp->b_pages)
+               _xfs_buf_free_pages(bp);
+
+       bp->b_pages = NULL;
+       bp->b_page_count = 0;
+       bp->b_addr = NULL;
+       bp->b_file_offset = 0;
+       bp->b_buffer_length = bp->b_count_desired = len;
+       bp->b_bn = XFS_BUF_DADDR_NULL;
+       bp->b_flags &= ~XBF_MAPPED;
+}
+
 static inline struct page *
 mem_to_page(
        void                    *addr)
index a9a1c451264513961573fbf2879fbc68c9a68e9a..50a7d5fb3b73dca40017a9b55252444f81c84a7b 100644 (file)
@@ -178,6 +178,7 @@ extern xfs_buf_t *xfs_buf_read(xfs_buftarg_t *, xfs_off_t, size_t,
                                xfs_buf_flags_t);
 
 extern xfs_buf_t *xfs_buf_get_empty(size_t, xfs_buftarg_t *);
+extern void xfs_buf_set_empty(struct xfs_buf *bp, size_t len);
 extern xfs_buf_t *xfs_buf_get_uncached(struct xfs_buftarg *, size_t, int);
 extern int xfs_buf_associate_memory(xfs_buf_t *, void *, size_t);
 extern void xfs_buf_hold(xfs_buf_t *);
index 18fd4beffcc3927351502abfd77daf645a155d37..211930246f2073f4759a569936b79ab387003e21 100644 (file)
@@ -1449,6 +1449,13 @@ xlog_dealloc_log(xlog_t *log)
 
        xlog_cil_destroy(log);
 
+       /*
+        * always need to ensure that the extra buffer does not point to memory
+        * owned by another log buffer before we free it.
+        */
+       xfs_buf_set_empty(log->l_xbuf, log->l_iclog_size);
+       xfs_buf_free(log->l_xbuf);
+
        iclog = log->l_iclog;
        for (i=0; i<log->l_iclog_bufs; i++) {
                xfs_buf_free(iclog->ic_bp);
@@ -1458,7 +1465,6 @@ xlog_dealloc_log(xlog_t *log)
        }
        spinlock_destroy(&log->l_icloglock);
 
-       xfs_buf_free(log->l_xbuf);
        log->l_mp->m_log = NULL;
        kmem_free(log);
 }      /* xlog_dealloc_log */
index 5cc464a17c93592f5d707552603d6b16705de333..04142caedb2bad62c158b08e6799e5f75bce5b4e 100644 (file)
@@ -204,6 +204,35 @@ xlog_bread(
        return 0;
 }
 
+/*
+ * Read at an offset into the buffer. Returns with the buffer in it's original
+ * state regardless of the result of the read.
+ */
+STATIC int
+xlog_bread_offset(
+       xlog_t          *log,
+       xfs_daddr_t     blk_no,         /* block to read from */
+       int             nbblks,         /* blocks to read */
+       xfs_buf_t       *bp,
+       xfs_caddr_t     offset)
+{
+       xfs_caddr_t     orig_offset = XFS_BUF_PTR(bp);
+       int             orig_len = bp->b_buffer_length;
+       int             error, error2;
+
+       error = XFS_BUF_SET_PTR(bp, offset, BBTOB(nbblks));
+       if (error)
+               return error;
+
+       error = xlog_bread_noalign(log, blk_no, nbblks, bp);
+
+       /* must reset buffer pointer even on error */
+       error2 = XFS_BUF_SET_PTR(bp, orig_offset, orig_len);
+       if (error)
+               return error;
+       return error2;
+}
+
 /*
  * Write out the buffer at the given block for the given number of blocks.
  * The buffer is kept locked across the write and is returned locked.
@@ -1229,20 +1258,12 @@ xlog_write_log_records(
                 */
                ealign = round_down(end_block, sectbb);
                if (j == 0 && (start_block + endcount > ealign)) {
-                       offset = XFS_BUF_PTR(bp);
-                       balign = BBTOB(ealign - start_block);
-                       error = XFS_BUF_SET_PTR(bp, offset + balign,
-                                               BBTOB(sectbb));
+                       offset = XFS_BUF_PTR(bp) + BBTOB(ealign - start_block);
+                       error = xlog_bread_offset(log, ealign, sectbb,
+                                                       bp, offset);
                        if (error)
                                break;
 
-                       error = xlog_bread_noalign(log, ealign, sectbb, bp);
-                       if (error)
-                               break;
-
-                       error = XFS_BUF_SET_PTR(bp, offset, bufblks);
-                       if (error)
-                               break;
                }
 
                offset = xlog_align(log, start_block, endcount, bp);
@@ -3448,19 +3469,9 @@ xlog_do_recovery_pass(
                                 *   - order is important.
                                 */
                                wrapped_hblks = hblks - split_hblks;
-                               error = XFS_BUF_SET_PTR(hbp,
-                                               offset + BBTOB(split_hblks),
-                                               BBTOB(hblks - split_hblks));
-                               if (error)
-                                       goto bread_err2;
-
-                               error = xlog_bread_noalign(log, 0,
-                                                          wrapped_hblks, hbp);
-                               if (error)
-                                       goto bread_err2;
-
-                               error = XFS_BUF_SET_PTR(hbp, offset,
-                                                       BBTOB(hblks));
+                               error = xlog_bread_offset(log, 0,
+                                               wrapped_hblks, hbp,
+                                               offset + BBTOB(split_hblks));
                                if (error)
                                        goto bread_err2;
                        }
@@ -3511,19 +3522,9 @@ xlog_do_recovery_pass(
                                 *   _first_, then the log start (LR header end)
                                 *   - order is important.
                                 */
-                               error = XFS_BUF_SET_PTR(dbp,
-                                               offset + BBTOB(split_bblks),
-                                               BBTOB(bblks - split_bblks));
-                               if (error)
-                                       goto bread_err2;
-
-                               error = xlog_bread_noalign(log, wrapped_hblks,
-                                               bblks - split_bblks,
-                                               dbp);
-                               if (error)
-                                       goto bread_err2;
-
-                               error = XFS_BUF_SET_PTR(dbp, offset, h_size);
+                               error = xlog_bread_offset(log, 0,
+                                               bblks - split_bblks, hbp,
+                                               offset + BBTOB(split_bblks));
                                if (error)
                                        goto bread_err2;
                        }