2006-08-01 07:23:08

by Jens Axboe

[permalink] [raw]
Subject: Re: [BLOCK] bh: Ensure bh fits within a page

On Tue, Aug 01 2006, Herbert Xu wrote:
> Hi Andrew:
>
> [BLOCK] bh: Ensure bh fits within a page
>
> There is a bug in jbd with slab debugging enabled where it was submitting
> a bh obtained via jbd_rep_kmalloc which crossed a page boundary. A lot
> of time was spent on tracking this down because the symptoms were far off
> from where the problem was.
>
> This patch adds a sanity check to submit_bh so we can immediately spot
> anyone doing similar things in future.
>
> Signed-off-by: Herbert Xu <[email protected]>
>
> While you're at it, could you fix that jbd bug for us :)
>
> Cheers,
> --
> Visit Openswan at http://www.openswan.org/
> Email: Herbert Xu ~{PmV>HI~} <[email protected]>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
> --
> diff --git a/fs/buffer.c b/fs/buffer.c
> index 71649ef..b998f08 100644
> --- a/fs/buffer.c
> +++ b/fs/buffer.c
> @@ -2790,6 +2790,7 @@ int submit_bh(int rw, struct buffer_head
> BUG_ON(!buffer_locked(bh));
> BUG_ON(!buffer_mapped(bh));
> BUG_ON(!bh->b_end_io);
> + WARN_ON(bh_offset(bh) + bh->b_size > PAGE_SIZE);

That looks really dangerous, I'd prefer that to be a BUG_ON() as well to
prevent nastiness further down.

--
Jens Axboe


2006-08-01 23:31:05

by Herbert Xu

[permalink] [raw]
Subject: Re: [BLOCK] bh: Ensure bh fits within a page

Jens Axboe <[email protected]> wrote:
>
> That looks really dangerous, I'd prefer that to be a BUG_ON() as well to
> prevent nastiness further down.

OK, I used a WARN_ON mainly because ext3 has been doing this for years
without killing anyone until now :)

[BLOCK] bh: Ensure bh fits within a page

There is a bug in jbd with slab debugging enabled where it was submitting
a bh obtained via jbd_rep_kmalloc which crossed a page boundary. A lot
of time was spent on tracking this down because the symptoms were far off
from where the problem was.

This patch adds a sanity check to submit_bh so we can immediately spot
anyone doing similar things in future.

Signed-off-by: Herbert Xu <[email protected]>

Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
diff --git a/fs/buffer.c b/fs/buffer.c
index 71649ef..ff34881 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -2790,6 +2790,7 @@ int submit_bh(int rw, struct buffer_head
BUG_ON(!buffer_locked(bh));
BUG_ON(!buffer_mapped(bh));
BUG_ON(!bh->b_end_io);
+ BUG_ON(bh_offset(bh) + bh->b_size > PAGE_SIZE);

if (buffer_ordered(bh) && (rw == WRITE))
rw = WRITE_BARRIER;

2006-08-01 23:32:28

by Herbert Xu

[permalink] [raw]
Subject: Re: [BLOCK] bh: Ensure bh fits within a page

On Wed, Aug 02, 2006 at 09:30:51AM +1000, Herbert Xu wrote:
>
> OK, I used a WARN_ON mainly because ext3 has been doing this for years
> without killing anyone until now :)
>
> [BLOCK] bh: Ensure bh fits within a page

Actually, the other reason is that we can't BUG_ON until ext3 is fixed
to not do this. So I suppose we should keep the WARN_ON until that
happens.

Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2006-08-01 23:44:22

by Andrew Morton

[permalink] [raw]
Subject: Re: [BLOCK] bh: Ensure bh fits within a page

On Wed, 02 Aug 2006 09:30:51 +1000
Herbert Xu <[email protected]> wrote:

> Jens Axboe <[email protected]> wrote:
> >
> > That looks really dangerous, I'd prefer that to be a BUG_ON() as well to
> > prevent nastiness further down.
>
> OK, I used a WARN_ON mainly because ext3 has been doing this for years
> without killing anyone until now :)

I can't apply either of these patches, because we _know_ it'll trigger.

Once the JBD fix is in hand, we can go with a WARN_ON_ONCE and if that is
all clear, then we can make it a BUG_ON().

2006-08-02 06:22:43

by Jens Axboe

[permalink] [raw]
Subject: Re: [BLOCK] bh: Ensure bh fits within a page

On Wed, Aug 02 2006, Herbert Xu wrote:
> On Wed, Aug 02, 2006 at 09:30:51AM +1000, Herbert Xu wrote:
> >
> > OK, I used a WARN_ON mainly because ext3 has been doing this for years
> > without killing anyone until now :)
> >
> > [BLOCK] bh: Ensure bh fits within a page
>
> Actually, the other reason is that we can't BUG_ON until ext3 is fixed
> to not do this. So I suppose we should keep the WARN_ON until that
> happens.

Yep, the ext3 bits need to go in first.

--
Jens Axboe

2006-08-17 23:11:18

by Badari Pulavarty

[permalink] [raw]
Subject: Re: [BLOCK] bh: Ensure bh fits within a page

On Wed, 2006-08-02 at 09:30 +1000, Herbert Xu wrote:
> Jens Axboe <[email protected]> wrote:
> >
> > That looks really dangerous, I'd prefer that to be a BUG_ON() as well to
> > prevent nastiness further down.
>
> OK, I used a WARN_ON mainly because ext3 has been doing this for years
> without killing anyone until now :)
>
> [BLOCK] bh: Ensure bh fits within a page
>
> There is a bug in jbd with slab debugging enabled where it was submitting
> a bh obtained via jbd_rep_kmalloc which crossed a page boundary. A lot
> of time was spent on tracking this down because the symptoms were far off
> from where the problem was.
>
> This patch adds a sanity check to submit_bh so we can immediately spot
> anyone doing similar things in future.
>
> Signed-off-by: Herbert Xu <[email protected]>
>
> Cheers,

Hi,

I am working on a fix to address this problem. Do you have any testcases
to reproduce the problem ? I have been running bunch of tests with
CONFIG_DEBUG_SLAB with WARN_ON() and I haven't seen it yet :(

Wondering, if there is any special thing I need to do to reproduce
the problem ? Please let me know.

Thanks,
Badari

2006-08-18 02:53:52

by Herbert Xu

[permalink] [raw]
Subject: Re: [BLOCK] bh: Ensure bh fits within a page

On Thu, Aug 17, 2006 at 04:14:16PM -0700, Badari Pulavarty wrote:
>
> Wondering, if there is any special thing I need to do to reproduce
> the problem ? Please let me know.

I must say that I didn't try to reproduce the problem. However,
here is the bugzilla entry for it. It apparently shows up during
installation:

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=200127

Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt