| Message ID | 20180723130414.47980-4-bfoster@redhat.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | xfs: embed dfops in the transaction | expand |
On Mon, Jul 23, 2018 at 09:04:02AM -0400, Brian Foster wrote: > The xattr remote value set/remove handlers both clear args.trans in > the error path without having cancelled the transaction. This leaks > the transaction, causes warnings around returning to userspace with > locks held and leads to system lockups or other general problems. > > The higher level xfs_attr_[set|remove]() functions already detect > and cancel args.trans when set in the error path. Drop the NULL > assignments from the rmtval handlers and allow the callers to clean > up the transaction correctly. > > Signed-off-by: Brian Foster <bfoster@redhat.com> > Reviewed-by: Christoph Hellwig <hch@lst.de> Looks good. Reviewed-by: Bill O'Donnell <billodo@redhat.com> > --- > fs/xfs/libxfs/xfs_attr_remote.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c > index 7841e6255129..829ab20f0cd7 100644 > --- a/fs/xfs/libxfs/xfs_attr_remote.c > +++ b/fs/xfs/libxfs/xfs_attr_remote.c > @@ -558,7 +558,6 @@ xfs_attr_rmtval_set( > return 0; > out_defer_cancel: > xfs_defer_cancel(args->trans->t_dfops); > - args->trans = NULL; > return error; > } > > @@ -646,6 +645,5 @@ xfs_attr_rmtval_remove( > return 0; > out_defer_cancel: > xfs_defer_cancel(args->trans->t_dfops); > - args->trans = NULL; > return error; > } > -- > 2.17.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jul 23, 2018 at 09:04:02AM -0400, Brian Foster wrote: > The xattr remote value set/remove handlers both clear args.trans in > the error path without having cancelled the transaction. This leaks > the transaction, causes warnings around returning to userspace with > locks held and leads to system lockups or other general problems. > > The higher level xfs_attr_[set|remove]() functions already detect > and cancel args.trans when set in the error path. Drop the NULL > assignments from the rmtval handlers and allow the callers to clean > up the transaction correctly. > > Signed-off-by: Brian Foster <bfoster@redhat.com> > Reviewed-by: Christoph Hellwig <hch@lst.de> Looks ok, Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> --D > --- > fs/xfs/libxfs/xfs_attr_remote.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c > index 7841e6255129..829ab20f0cd7 100644 > --- a/fs/xfs/libxfs/xfs_attr_remote.c > +++ b/fs/xfs/libxfs/xfs_attr_remote.c > @@ -558,7 +558,6 @@ xfs_attr_rmtval_set( > return 0; > out_defer_cancel: > xfs_defer_cancel(args->trans->t_dfops); > - args->trans = NULL; > return error; > } > > @@ -646,6 +645,5 @@ xfs_attr_rmtval_remove( > return 0; > out_defer_cancel: > xfs_defer_cancel(args->trans->t_dfops); > - args->trans = NULL; > return error; > } > -- > 2.17.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c index 7841e6255129..829ab20f0cd7 100644 --- a/fs/xfs/libxfs/xfs_attr_remote.c +++ b/fs/xfs/libxfs/xfs_attr_remote.c @@ -558,7 +558,6 @@ xfs_attr_rmtval_set( return 0; out_defer_cancel: xfs_defer_cancel(args->trans->t_dfops); - args->trans = NULL; return error; } @@ -646,6 +645,5 @@ xfs_attr_rmtval_remove( return 0; out_defer_cancel: xfs_defer_cancel(args->trans->t_dfops); - args->trans = NULL; return error; }