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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <AANLkTinVpZoQ9x5O4v5A718LeWzVBsFUiZgHc9FwoRKH@mail.gmail.com>
Date:	Sat, 30 Oct 2010 16:15:44 +0100
From:	Daniel J Blueman <daniel.blueman@...il.com>
To:	Chris Mason <chris.mason@...cle.com>
Cc:	Linux BTRFS <linux-btrfs@...r.kernel.org>,
	Josef Bacik <josef@...hat.com>,
	Linux Kernel <linux-kernel@...r.kernel.org>
Subject: [2.6.37-rc0 patch] direct I/O submission fixes v3

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


On 25 July 2010 15:42, Josef Bacik <josef@...hat.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 <josef@...hat.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.blueman@...il.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-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/