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@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.blueman@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.blueman@gmail.com>
Date: 25 July 2010 19:53
Subject: Re: [2.6.35-rc6 patch] direct I/O submission fixes v2
To: Josef Bacik <josef@redhat.com>, Chris Mason
<chris.mason@oracle.com>
Cc: Linux BTRFS <linux-btrfs@vger.kernel.org>
On 25 July 2010 15:42, Josef Bacik <josef@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 <josef@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.blueman@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 majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html