[2.6.37-rc0 patch] direct I/O submission fixes v3

[Daniel J Blueman]

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 <josef@xxxxxxxxxx>

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.blueman@xxxxxxxxx>

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.blueman@xxxxxxxxx>
Date: 25 July 2010 19:53
Subject: Re: [2.6.35-rc6 patch] direct I/O submission fixes v2
To: Josef Bacik <josef@xxxxxxxxxx>, Chris Mason <chris.mason@xxxxxxxxxx>
Cc: Linux BTRFS <linux-btrfs@xxxxxxxxxxxxxxx>


On 25 July 2010 15:42, Josef Bacik <josef@xxxxxxxxxx> 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 <josef@xxxxxxxxxx>
>
> 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.blueman@xxxxxxxxx>

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-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/