[Devel,RH7] vfs: add warning in guard_bio_eod() if truncated_bytes > bvec->bv_len

Submitted by Pavel Tikhomirov on Dec. 2, 2016, 3:49 p.m.

Details

Message ID 20161202154930.11384-1-ptikhomirov@virtuozzo.com
State New
Series "vfs: add warning in guard_bio_eod() if truncated_bytes > bvec->bv_len"
Headers show

Commit Message

Pavel Tikhomirov Dec. 2, 2016, 3:49 p.m.
https://jira.sw.ru/browse/PSBM-55105

In bug we crashed in zero_fill_bio when trying to zero memset bio_vec:

struct bio_vec {
  bv_page = 0xffffea0004437500,
  bv_len = 4294948864,
  bv_offset = 0
}

which is bigger than its bio->bi_size = 104448, guard_bio_eod might
lead to these bv_len overflow and is suspicious as quiet recently
in vz7.19.4 we've ported commit 2573b2539875("vfs: make guard_bh_eod()
more generic") which adds bv_len reduction, and before that there
were no crash.

Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
---
 fs/buffer.c | 1 +
 1 file changed, 1 insertion(+)

Patch hide | download patch | download mbox

diff --git a/fs/buffer.c b/fs/buffer.c
index c45200d..b820080 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -3009,6 +3009,7 @@  void guard_bio_eod(int rw, struct bio *bio)
 
 	/* Truncate the bio.. */
 	bio->bi_size -= truncated_bytes;
+	WARN_ON(truncated_bytes > bvec->bv_len);
 	bvec->bv_len -= truncated_bytes;
 
 	/* ..and clear the end of the buffer for reads */

Comments

Dmitry Monakhov Dec. 3, 2016, 8:22 a.m.
Pavel Tikhomirov <ptikhomirov@virtuozzo.com> writes:

> https://jira.sw.ru/browse/PSBM-55105
>
> In bug we crashed in zero_fill_bio when trying to zero memset bio_vec:
>
> struct bio_vec {
>   bv_page = 0xffffea0004437500,
>   bv_len = 4294948864,
>   bv_offset = 0
> }
>
> which is bigger than its bio->bi_size = 104448, guard_bio_eod might
> lead to these bv_len overflow and is suspicious as quiet recently
> in vz7.19.4 we've ported commit 2573b2539875("vfs: make guard_bh_eod()
> more generic") which adds bv_len reduction, and before that there
> were no crash.
>
> Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
> ---
>  fs/buffer.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/fs/buffer.c b/fs/buffer.c
> index c45200d..b820080 100644
> --- a/fs/buffer.c
> +++ b/fs/buffer.c
> @@ -3009,6 +3009,7 @@ void guard_bio_eod(int rw, struct bio *bio)
>  
>  	/* Truncate the bio.. */
>  	bio->bi_size -= truncated_bytes;
> +	WARN_ON(truncated_bytes > bvec->bv_len);
BUG_ON would be more appropriate here.
>  	bvec->bv_len -= truncated_bytes;
>  
>  	/* ..and clear the end of the buffer for reads */
> -- 
> 2.9.3
Roman Kagan Dec. 5, 2016, 6:37 a.m.
On Sat, Dec 03, 2016 at 11:22:26AM +0300, Dmitry Monakhov wrote:
> 
> Pavel Tikhomirov <ptikhomirov@virtuozzo.com> writes:
> 
> > https://jira.sw.ru/browse/PSBM-55105
> >
> > In bug we crashed in zero_fill_bio when trying to zero memset bio_vec:
> >
> > struct bio_vec {
> >   bv_page = 0xffffea0004437500,
> >   bv_len = 4294948864,
> >   bv_offset = 0
> > }
> >
> > which is bigger than its bio->bi_size = 104448, guard_bio_eod might
> > lead to these bv_len overflow and is suspicious as quiet recently
> > in vz7.19.4 we've ported commit 2573b2539875("vfs: make guard_bh_eod()
> > more generic") which adds bv_len reduction, and before that there
> > were no crash.
> >
> > Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
> > ---
> >  fs/buffer.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/fs/buffer.c b/fs/buffer.c
> > index c45200d..b820080 100644
> > --- a/fs/buffer.c
> > +++ b/fs/buffer.c
> > @@ -3009,6 +3009,7 @@ void guard_bio_eod(int rw, struct bio *bio)
> >  
> >  	/* Truncate the bio.. */
> >  	bio->bi_size -= truncated_bytes;
> > +	WARN_ON(truncated_bytes > bvec->bv_len);
> BUG_ON would be more appropriate here.

Haven't you seen how Linus "critisized" Andrew for doing exactly that?

Roman.