aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDave Chinner <david@fromorbit.com>2008-11-17 01:37:10 -0500
committerLachlan McIlroy <lachlan@redback.melbourne.sgi.com>2008-11-17 01:37:10 -0500
commitcc09c0dc57de7f7d2ed89d480b5653e5f6a32f2c (patch)
tree3c6145ccb00f603c47e020cfa45f159ab76cf9bf
parent6307091fe69ae74747298bdcaf43119ad67bda3a (diff)
[XFS] Fix double free of log tickets
When an I/O error occurs during an intermediate commit on a rolling transaction, xfs_trans_commit() will free the transaction structure and the related ticket. However, the duplicate transaction that gets used as the transaction continues still contains a pointer to the ticket. Hence when the duplicate transaction is cancelled and freed, we free the ticket a second time. Add reference counting to the ticket so that we hold an extra reference to the ticket over the transaction commit. We drop the extra reference once we have checked that the transaction commit did not return an error, thus avoiding a double free on commit error. Credit to Nick Piggin for tripping over the problem. SGI-PV: 989741 Signed-off-by: Dave Chinner <david@fromorbit.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Lachlan McIlroy <lachlan@sgi.com>
-rw-r--r--fs/xfs/xfs_bmap.c10
-rw-r--r--fs/xfs/xfs_inode.c10
-rw-r--r--fs/xfs/xfs_log.c39
-rw-r--r--fs/xfs/xfs_log.h4
-rw-r--r--fs/xfs/xfs_log_priv.h1
-rw-r--r--fs/xfs/xfs_trans.c9
-rw-r--r--fs/xfs/xfs_utils.c6
-rw-r--r--fs/xfs/xfs_vnodeops.c6
8 files changed, 66 insertions, 19 deletions
diff --git a/fs/xfs/xfs_bmap.c b/fs/xfs/xfs_bmap.c
index db289050692f..c3912213645c 100644
--- a/fs/xfs/xfs_bmap.c
+++ b/fs/xfs/xfs_bmap.c
@@ -4292,9 +4292,15 @@ xfs_bmap_finish(
4292 * We have a new transaction, so we should return committed=1, 4292 * We have a new transaction, so we should return committed=1,
4293 * even though we're returning an error. 4293 * even though we're returning an error.
4294 */ 4294 */
4295 if (error) { 4295 if (error)
4296 return error; 4296 return error;
4297 } 4297
4298 /*
4299 * transaction commit worked ok so we can drop the extra ticket
4300 * reference that we gained in xfs_trans_dup()
4301 */
4302 xfs_log_ticket_put(ntp->t_ticket);
4303
4298 if ((error = xfs_trans_reserve(ntp, 0, logres, 0, XFS_TRANS_PERM_LOG_RES, 4304 if ((error = xfs_trans_reserve(ntp, 0, logres, 0, XFS_TRANS_PERM_LOG_RES,
4299 logcount))) 4305 logcount)))
4300 return error; 4306 return error;
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index cd522827f99e..b97710047062 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -1782,8 +1782,14 @@ xfs_itruncate_finish(
1782 xfs_trans_ijoin(ntp, ip, XFS_ILOCK_EXCL | XFS_IOLOCK_EXCL); 1782 xfs_trans_ijoin(ntp, ip, XFS_ILOCK_EXCL | XFS_IOLOCK_EXCL);
1783 xfs_trans_ihold(ntp, ip); 1783 xfs_trans_ihold(ntp, ip);
1784 1784
1785 if (!error) 1785 if (error)
1786 error = xfs_trans_reserve(ntp, 0, 1786 return error;
1787 /*
1788 * transaction commit worked ok so we can drop the extra ticket
1789 * reference that we gained in xfs_trans_dup()
1790 */
1791 xfs_log_ticket_put(ntp->t_ticket);
1792 error = xfs_trans_reserve(ntp, 0,
1787 XFS_ITRUNCATE_LOG_RES(mp), 0, 1793 XFS_ITRUNCATE_LOG_RES(mp), 0,
1788 XFS_TRANS_PERM_LOG_RES, 1794 XFS_TRANS_PERM_LOG_RES,
1789 XFS_ITRUNCATE_LOG_COUNT); 1795 XFS_ITRUNCATE_LOG_COUNT);
diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index 92c20a8d9e69..4bf44aef644c 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -100,12 +100,11 @@ STATIC void xlog_ungrant_log_space(xlog_t *log,
100 100
101 101
102/* local ticket functions */ 102/* local ticket functions */
103STATIC xlog_ticket_t *xlog_ticket_get(xlog_t *log, 103STATIC xlog_ticket_t *xlog_ticket_alloc(xlog_t *log,
104 int unit_bytes, 104 int unit_bytes,
105 int count, 105 int count,
106 char clientid, 106 char clientid,
107 uint flags); 107 uint flags);
108STATIC void xlog_ticket_put(xlog_t *log, xlog_ticket_t *ticket);
109 108
110#if defined(DEBUG) 109#if defined(DEBUG)
111STATIC void xlog_verify_dest_ptr(xlog_t *log, __psint_t ptr); 110STATIC void xlog_verify_dest_ptr(xlog_t *log, __psint_t ptr);
@@ -360,7 +359,7 @@ xfs_log_done(xfs_mount_t *mp,
360 */ 359 */
361 xlog_trace_loggrant(log, ticket, "xfs_log_done: (non-permanent)"); 360 xlog_trace_loggrant(log, ticket, "xfs_log_done: (non-permanent)");
362 xlog_ungrant_log_space(log, ticket); 361 xlog_ungrant_log_space(log, ticket);
363 xlog_ticket_put(log, ticket); 362 xfs_log_ticket_put(ticket);
364 } else { 363 } else {
365 xlog_trace_loggrant(log, ticket, "xfs_log_done: (permanent)"); 364 xlog_trace_loggrant(log, ticket, "xfs_log_done: (permanent)");
366 xlog_regrant_reserve_log_space(log, ticket); 365 xlog_regrant_reserve_log_space(log, ticket);
@@ -514,7 +513,7 @@ xfs_log_reserve(xfs_mount_t *mp,
514 retval = xlog_regrant_write_log_space(log, internal_ticket); 513 retval = xlog_regrant_write_log_space(log, internal_ticket);
515 } else { 514 } else {
516 /* may sleep if need to allocate more tickets */ 515 /* may sleep if need to allocate more tickets */
517 internal_ticket = xlog_ticket_get(log, unit_bytes, cnt, 516 internal_ticket = xlog_ticket_alloc(log, unit_bytes, cnt,
518 client, flags); 517 client, flags);
519 if (!internal_ticket) 518 if (!internal_ticket)
520 return XFS_ERROR(ENOMEM); 519 return XFS_ERROR(ENOMEM);
@@ -749,7 +748,7 @@ xfs_log_unmount_write(xfs_mount_t *mp)
749 if (tic) { 748 if (tic) {
750 xlog_trace_loggrant(log, tic, "unmount rec"); 749 xlog_trace_loggrant(log, tic, "unmount rec");
751 xlog_ungrant_log_space(log, tic); 750 xlog_ungrant_log_space(log, tic);
752 xlog_ticket_put(log, tic); 751 xfs_log_ticket_put(tic);
753 } 752 }
754 } else { 753 } else {
755 /* 754 /*
@@ -3222,22 +3221,33 @@ xlog_state_want_sync(xlog_t *log, xlog_in_core_t *iclog)
3222 */ 3221 */
3223 3222
3224/* 3223/*
3225 * Free a used ticket. 3224 * Free a used ticket when it's refcount falls to zero.
3226 */ 3225 */
3227STATIC void 3226void
3228xlog_ticket_put(xlog_t *log, 3227xfs_log_ticket_put(
3229 xlog_ticket_t *ticket) 3228 xlog_ticket_t *ticket)
3230{ 3229{
3231 sv_destroy(&ticket->t_wait); 3230 ASSERT(atomic_read(&ticket->t_ref) > 0);
3232 kmem_zone_free(xfs_log_ticket_zone, ticket); 3231 if (atomic_dec_and_test(&ticket->t_ref)) {
3233} /* xlog_ticket_put */ 3232 sv_destroy(&ticket->t_wait);
3233 kmem_zone_free(xfs_log_ticket_zone, ticket);
3234 }
3235}
3234 3236
3237xlog_ticket_t *
3238xfs_log_ticket_get(
3239 xlog_ticket_t *ticket)
3240{
3241 ASSERT(atomic_read(&ticket->t_ref) > 0);
3242 atomic_inc(&ticket->t_ref);
3243 return ticket;
3244}
3235 3245
3236/* 3246/*
3237 * Allocate and initialise a new log ticket. 3247 * Allocate and initialise a new log ticket.
3238 */ 3248 */
3239STATIC xlog_ticket_t * 3249STATIC xlog_ticket_t *
3240xlog_ticket_get(xlog_t *log, 3250xlog_ticket_alloc(xlog_t *log,
3241 int unit_bytes, 3251 int unit_bytes,
3242 int cnt, 3252 int cnt,
3243 char client, 3253 char client,
@@ -3308,6 +3318,7 @@ xlog_ticket_get(xlog_t *log,
3308 unit_bytes += 2*BBSIZE; 3318 unit_bytes += 2*BBSIZE;
3309 } 3319 }
3310 3320
3321 atomic_set(&tic->t_ref, 1);
3311 tic->t_unit_res = unit_bytes; 3322 tic->t_unit_res = unit_bytes;
3312 tic->t_curr_res = unit_bytes; 3323 tic->t_curr_res = unit_bytes;
3313 tic->t_cnt = cnt; 3324 tic->t_cnt = cnt;
@@ -3323,7 +3334,7 @@ xlog_ticket_get(xlog_t *log,
3323 xlog_tic_reset_res(tic); 3334 xlog_tic_reset_res(tic);
3324 3335
3325 return tic; 3336 return tic;
3326} /* xlog_ticket_get */ 3337}
3327 3338
3328 3339
3329/****************************************************************************** 3340/******************************************************************************
diff --git a/fs/xfs/xfs_log.h b/fs/xfs/xfs_log.h
index d47b91f10822..8a3e84e900a3 100644
--- a/fs/xfs/xfs_log.h
+++ b/fs/xfs/xfs_log.h
@@ -134,6 +134,7 @@ typedef struct xfs_log_callback {
134#ifdef __KERNEL__ 134#ifdef __KERNEL__
135/* Log manager interfaces */ 135/* Log manager interfaces */
136struct xfs_mount; 136struct xfs_mount;
137struct xlog_ticket;
137xfs_lsn_t xfs_log_done(struct xfs_mount *mp, 138xfs_lsn_t xfs_log_done(struct xfs_mount *mp,
138 xfs_log_ticket_t ticket, 139 xfs_log_ticket_t ticket,
139 void **iclog, 140 void **iclog,
@@ -177,6 +178,9 @@ int xfs_log_need_covered(struct xfs_mount *mp);
177 178
178void xlog_iodone(struct xfs_buf *); 179void xlog_iodone(struct xfs_buf *);
179 180
181struct xlog_ticket * xfs_log_ticket_get(struct xlog_ticket *ticket);
182void xfs_log_ticket_put(struct xlog_ticket *ticket);
183
180#endif 184#endif
181 185
182 186
diff --git a/fs/xfs/xfs_log_priv.h b/fs/xfs/xfs_log_priv.h
index de7ef6ca9206..b39a1980e82d 100644
--- a/fs/xfs/xfs_log_priv.h
+++ b/fs/xfs/xfs_log_priv.h
@@ -245,6 +245,7 @@ typedef struct xlog_ticket {
245 struct xlog_ticket *t_next; /* :4|8 */ 245 struct xlog_ticket *t_next; /* :4|8 */
246 struct xlog_ticket *t_prev; /* :4|8 */ 246 struct xlog_ticket *t_prev; /* :4|8 */
247 xlog_tid_t t_tid; /* transaction identifier : 4 */ 247 xlog_tid_t t_tid; /* transaction identifier : 4 */
248 atomic_t t_ref; /* ticket reference count : 4 */
248 int t_curr_res; /* current reservation in bytes : 4 */ 249 int t_curr_res; /* current reservation in bytes : 4 */
249 int t_unit_res; /* unit reservation in bytes : 4 */ 250 int t_unit_res; /* unit reservation in bytes : 4 */
250 char t_ocnt; /* original count : 1 */ 251 char t_ocnt; /* original count : 1 */
diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
index ad137efc8702..8570b826fedd 100644
--- a/fs/xfs/xfs_trans.c
+++ b/fs/xfs/xfs_trans.c
@@ -290,7 +290,7 @@ xfs_trans_dup(
290 ASSERT(tp->t_ticket != NULL); 290 ASSERT(tp->t_ticket != NULL);
291 291
292 ntp->t_flags = XFS_TRANS_PERM_LOG_RES | (tp->t_flags & XFS_TRANS_RESERVE); 292 ntp->t_flags = XFS_TRANS_PERM_LOG_RES | (tp->t_flags & XFS_TRANS_RESERVE);
293 ntp->t_ticket = tp->t_ticket; 293 ntp->t_ticket = xfs_log_ticket_get(tp->t_ticket);
294 ntp->t_blk_res = tp->t_blk_res - tp->t_blk_res_used; 294 ntp->t_blk_res = tp->t_blk_res - tp->t_blk_res_used;
295 tp->t_blk_res = tp->t_blk_res_used; 295 tp->t_blk_res = tp->t_blk_res_used;
296 ntp->t_rtx_res = tp->t_rtx_res - tp->t_rtx_res_used; 296 ntp->t_rtx_res = tp->t_rtx_res - tp->t_rtx_res_used;
@@ -1260,6 +1260,13 @@ xfs_trans_roll(
1260 trans = *tpp; 1260 trans = *tpp;
1261 1261
1262 /* 1262 /*
1263 * transaction commit worked ok so we can drop the extra ticket
1264 * reference that we gained in xfs_trans_dup()
1265 */
1266 xfs_log_ticket_put(trans->t_ticket);
1267
1268
1269 /*
1263 * Reserve space in the log for th next transaction. 1270 * Reserve space in the log for th next transaction.
1264 * This also pushes items in the "AIL", the list of logged items, 1271 * This also pushes items in the "AIL", the list of logged items,
1265 * out to disk if they are taking up space at the tail of the log 1272 * out to disk if they are taking up space at the tail of the log
diff --git a/fs/xfs/xfs_utils.c b/fs/xfs/xfs_utils.c
index 35d4d414bcc2..771144932ab4 100644
--- a/fs/xfs/xfs_utils.c
+++ b/fs/xfs/xfs_utils.c
@@ -172,6 +172,12 @@ xfs_dir_ialloc(
172 *ipp = NULL; 172 *ipp = NULL;
173 return code; 173 return code;
174 } 174 }
175
176 /*
177 * transaction commit worked ok so we can drop the extra ticket
178 * reference that we gained in xfs_trans_dup()
179 */
180 xfs_log_ticket_put(tp->t_ticket);
175 code = xfs_trans_reserve(tp, 0, log_res, 0, 181 code = xfs_trans_reserve(tp, 0, log_res, 0,
176 XFS_TRANS_PERM_LOG_RES, log_count); 182 XFS_TRANS_PERM_LOG_RES, log_count);
177 /* 183 /*
diff --git a/fs/xfs/xfs_vnodeops.c b/fs/xfs/xfs_vnodeops.c
index c45ea278ef41..0574aadc4d3c 100644
--- a/fs/xfs/xfs_vnodeops.c
+++ b/fs/xfs/xfs_vnodeops.c
@@ -1019,6 +1019,12 @@ xfs_inactive_symlink_rmt(
1019 goto error0; 1019 goto error0;
1020 } 1020 }
1021 /* 1021 /*
1022 * transaction commit worked ok so we can drop the extra ticket
1023 * reference that we gained in xfs_trans_dup()
1024 */
1025 xfs_log_ticket_put(tp->t_ticket);
1026
1027 /*
1022 * Remove the memory for extent descriptions (just bookkeeping). 1028 * Remove the memory for extent descriptions (just bookkeeping).
1023 */ 1029 */
1024 if (ip->i_df.if_bytes) 1030 if (ip->i_df.if_bytes)