Hi Chris, These patches from myself and Josef are still relevant, but not in your last mainline pull request.
Can you add them if you are happy please? I've rediffed them [1,2] against your for-linux tree. Many thanks, Daniel --- [1] Fix use-after-free on error path. Signed-off-by: Josef Bacik <jo...@redhat.com> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index 558cac2..986cc40 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -5761,7 +5761,7 @@ free_ordered: if (write) { struct btrfs_ordered_extent *ordered; ordered = btrfs_lookup_ordered_extent(inode, - dip->logical_offset); + file_offset); if (!test_bit(BTRFS_ORDERED_PREALLOC, &ordered->flags) && !test_bit(BTRFS_ORDERED_NOCOW, &ordered->flags)) btrfs_free_reserved_extent(root, ordered->start, --- [2] Fix leak of 'dip' on error path and unnecessary double-assignment. Signed-off-by: Daniel J Blueman <daniel.blue...@gmail.com> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index 558cac2..312eeb7 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -5701,15 +5701,15 @@ static void btrfs_submit_direct(int rw, struct bio *bio, struct inode *inode, ret = -ENOMEM; goto free_ordered; } - dip->csums = NULL; if (!skip_sum) { dip->csums = kmalloc(sizeof(u32) * bio->bi_vcnt, GFP_NOFS); if (!dip->csums) { ret = -ENOMEM; - goto free_ordered; + goto out_err; } - } + } else + dip->csums = NULL; dip->private = bio->bi_private; dip->inode = inode; ---------- Forwarded message ---------- From: Daniel J Blueman <daniel.blue...@gmail.com> Date: 25 July 2010 19:53 Subject: Re: [2.6.35-rc6 patch] direct I/O submission fixes v2 To: Josef Bacik <jo...@redhat.com>, Chris Mason <chris.ma...@oracle.com> Cc: Linux BTRFS <linux-btrfs@vger.kernel.org> On 25 July 2010 15:42, Josef Bacik <jo...@redhat.com> wrote: > On Sat, Jul 24, 2010 at 12:01:59AM +0100, Daniel J Blueman wrote: >> Hi Chris, >> >> This fixes some issues relating to direct I/O submission, however a >> further patch will be needed to handle the case where allocation of >> 'dip' fails, which is always dereferenced when finding the ordered >> extent. >> > > Hi, > > There's an easier way to do this. This patch should fix the problem, > > Signed-off-by: Josef Bacik <jo...@redhat.com> > > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c > index 3232945..7259ef9 100644 > --- a/fs/btrfs/inode.c > +++ b/fs/btrfs/inode.c > @@ -5815,7 +5815,7 @@ free_ordered: > if (write) { > struct btrfs_ordered_extent *ordered; > ordered = btrfs_lookup_ordered_extent(inode, > - dip->logical_offset); > + file_offset); > if (!test_bit(BTRFS_ORDERED_PREALLOC, &ordered->flags) && > !test_bit(BTRFS_ORDERED_NOCOW, &ordered->flags)) > btrfs_free_reserved_extent(root, ordered->start, > Good move! With your patch applied, mine (now not priority) then becomes: Fix leak of 'dip' on error path and double assignment. Signed-off-by: Daniel J Blueman <daniel.blue...@gmail.com> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index 1bff92a..bd7f940 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -5652,15 +5652,15 @@ static void btrfs_submit_direct(int rw, struct bio *bio, struct inode *inode, ret = -ENOMEM; goto free_ordered; } - dip->csums = NULL; if (!skip_sum) { dip->csums = kmalloc(sizeof(u32) * bio->bi_vcnt, GFP_NOFS); if (!dip->csums) { ret = -ENOMEM; - goto free_ordered; + goto out_err; } - } + } else + dip->csums = NULL; dip->private = bio->bi_private; dip->inode = inode; -- Daniel J Blueman -- 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