Hi Alex, On 02/06/13 12:18, Alex Lyakas wrote: > Hi Jan, Arne, > I see this code in create_snapshot: > > if (inherit) { > pending_snapshot->inherit = *inherit; > *inherit = NULL; /* take responsibility to free it */ > } > > So, first thing I think it should be: > if (*inherit) > because in btrfs_ioctl_snap_create_v2() we have: > struct btrfs_qgroup_inherit *inherit = NULL; > ... > btrfs_ioctl_snap_create_transid(..., &inherit) > > so the current check is very unlikely to be NULL.
But in btrfs_ioctl_snap_create it is called with NULL, so *inherit would dereference a NULL pointer. > > Second, I don't see anybody freeing pending_snapshot->inherit. I guess > it should be freed after callin btrfs_qgroup_inherit() and also in > btrfs_destroy_pending_snapshots(). You're right. In our original version (6f72c7e20dbaea5) it was still there, in transaction.c. It has been removed in 6fa9700e734: commit 6fa9700e734275de2acbcb0e99414bd7ddfc60f1 Author: Miao Xie <mi...@cn.fujitsu.com> Date: Thu Sep 6 04:00:32 2012 -0600 Btrfs: fix error path in create_pending_snapshot() This patch fixes the following problem: - If we failed to deal with the delayed dir items, we should abort transaction, just as its comment said. Fix it. - If root reference or root back reference insertion failed, we should abort transaction. Fix it. - Fix the double free problem of pending->inherit. - Do not restore the trans->rsv if we doesn't change it. - make the error path more clearly. Signed-off-by: Miao Xie <mi...@cn.fujitsu.com> Miao, can you please explain where you see a double free? -Arne > Thanks, > Alex. > -- > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html