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 <miaox@xxxxxxxxxxxxxx>
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 <miaox@xxxxxxxxxxxxxx>
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 majordomo@xxxxxxxxxxxxxxx
> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
