* [PATCH] bcache: fix BUG_ON due to integer overflow with GC_SECTORS_USED
@ 2014-02-11 18:01 Darrick J. Wong
0 siblings, 0 replies; 6+ messages in thread
From: Darrick J. Wong @ 2014-02-11 18:01 UTC (permalink / raw)
To: linux-stable; +Cc: linux-bcache
The BUG_ON at the end of __bch_btree_mark_key can be triggered due to
an integer overflow error:
BITMASK(GC_SECTORS_USED, struct bucket, gc_mark, 2, 13);
...
SET_GC_SECTORS_USED(g, min_t(unsigned,
GC_SECTORS_USED(g) + KEY_SIZE(k),
(1 << 14) - 1));
BUG_ON(!GC_SECTORS_USED(g));
In bcache.h, the SECTORS_USED bitfield is defined to be 13 bits wide.
While the SET_ code tries to ensure that the field doesn't overflow by
clamping it to (1<<14)-1 == 16383, this is incorrect because 16383
requires 14 bits. Therefore, if GC_SECTORS_USED() + KEY_SIZE() =
8192, the SET_ statement tries to store 8192 into a 13-bit field. In
a 13-bit field, 8192 becomes zero, thus triggering the BUG_ON.
Therefore, create a field width constant and a max value constant, and
use those to create the bitfield and check the inputs to
SET_GC_SECTORS_USED. Arguably the BITMASK() template ought to have
BUG_ON checks for too-large values, but that's a separate patch.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Kent Overstreet <kmo@daterainc.com>
---
Greg, stable team et. all - can you apply this asap to 3.13? It hasn't hit
Linus's tree yet (it's on its way via Jens), but multiple users are hitting this
BUG_ON() and the fix has been tested.
drivers/md/bcache/bcache.h | 4 +++-
drivers/md/bcache/btree.c | 2 +-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/md/bcache/bcache.h b/drivers/md/bcache/bcache.h
index 0c707e4f4e..a4c7306ff4 100644
--- a/drivers/md/bcache/bcache.h
+++ b/drivers/md/bcache/bcache.h
@@ -210,7 +210,9 @@ BITMASK(GC_MARK, struct bucket, gc_mark, 0, 2);
#define GC_MARK_RECLAIMABLE 0
#define GC_MARK_DIRTY 1
#define GC_MARK_METADATA 2
-BITMASK(GC_SECTORS_USED, struct bucket, gc_mark, 2, 13);
+#define GC_SECTORS_USED_SIZE 13
+#define MAX_GC_SECTORS_USED (~(~0ULL << GC_SECTORS_USED_SIZE))
+BITMASK(GC_SECTORS_USED, struct bucket, gc_mark, 2, GC_SECTORS_USED_SIZE);
BITMASK(GC_MOVE, struct bucket, gc_mark, 15, 1);
#include "journal.h"
diff --git a/drivers/md/bcache/btree.c b/drivers/md/bcache/btree.c
index 98cc0a810a..7d421180b6 100644
--- a/drivers/md/bcache/btree.c
+++ b/drivers/md/bcache/btree.c
@@ -1167,7 +1167,7 @@ uint8_t __bch_btree_mark_key(struct cache_set *c, int level, struct bkey *k)
/* guard against overflow */
SET_GC_SECTORS_USED(g, min_t(unsigned,
GC_SECTORS_USED(g) + KEY_SIZE(k),
- (1 << 14) - 1));
+ MAX_GC_SECTORS_USED));
BUG_ON(!GC_SECTORS_USED(g));
}
--
1.9.0.rc3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH] bcache: fix BUG_ON due to integer overflow with GC_SECTORS_USED
@ 2014-02-11 18:02 Darrick J. Wong
2014-02-11 18:14 ` Greg KH
0 siblings, 1 reply; 6+ messages in thread
From: Darrick J. Wong @ 2014-02-11 18:02 UTC (permalink / raw)
To: stable; +Cc: linux-bcache
The BUG_ON at the end of __bch_btree_mark_key can be triggered due to
an integer overflow error:
BITMASK(GC_SECTORS_USED, struct bucket, gc_mark, 2, 13);
...
SET_GC_SECTORS_USED(g, min_t(unsigned,
GC_SECTORS_USED(g) + KEY_SIZE(k),
(1 << 14) - 1));
BUG_ON(!GC_SECTORS_USED(g));
In bcache.h, the SECTORS_USED bitfield is defined to be 13 bits wide.
While the SET_ code tries to ensure that the field doesn't overflow by
clamping it to (1<<14)-1 == 16383, this is incorrect because 16383
requires 14 bits. Therefore, if GC_SECTORS_USED() + KEY_SIZE() =
8192, the SET_ statement tries to store 8192 into a 13-bit field. In
a 13-bit field, 8192 becomes zero, thus triggering the BUG_ON.
Therefore, create a field width constant and a max value constant, and
use those to create the bitfield and check the inputs to
SET_GC_SECTORS_USED. Arguably the BITMASK() template ought to have
BUG_ON checks for too-large values, but that's a separate patch.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Kent Overstreet <kmo@daterainc.com>
---
Greg, stable team et. all - can you apply this asap to 3.13? It hasn't hit
Linus's tree yet (it's on its way via Jens), but multiple users are hitting this
BUG_ON() and the fix has been tested.
drivers/md/bcache/bcache.h | 4 +++-
drivers/md/bcache/btree.c | 2 +-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/md/bcache/bcache.h b/drivers/md/bcache/bcache.h
index 0c707e4f4e..a4c7306ff4 100644
--- a/drivers/md/bcache/bcache.h
+++ b/drivers/md/bcache/bcache.h
@@ -210,7 +210,9 @@ BITMASK(GC_MARK, struct bucket, gc_mark, 0, 2);
#define GC_MARK_RECLAIMABLE 0
#define GC_MARK_DIRTY 1
#define GC_MARK_METADATA 2
-BITMASK(GC_SECTORS_USED, struct bucket, gc_mark, 2, 13);
+#define GC_SECTORS_USED_SIZE 13
+#define MAX_GC_SECTORS_USED (~(~0ULL << GC_SECTORS_USED_SIZE))
+BITMASK(GC_SECTORS_USED, struct bucket, gc_mark, 2, GC_SECTORS_USED_SIZE);
BITMASK(GC_MOVE, struct bucket, gc_mark, 15, 1);
#include "journal.h"
diff --git a/drivers/md/bcache/btree.c b/drivers/md/bcache/btree.c
index 98cc0a810a..7d421180b6 100644
--- a/drivers/md/bcache/btree.c
+++ b/drivers/md/bcache/btree.c
@@ -1167,7 +1167,7 @@ uint8_t __bch_btree_mark_key(struct cache_set *c, int level, struct bkey *k)
/* guard against overflow */
SET_GC_SECTORS_USED(g, min_t(unsigned,
GC_SECTORS_USED(g) + KEY_SIZE(k),
- (1 << 14) - 1));
+ MAX_GC_SECTORS_USED));
BUG_ON(!GC_SECTORS_USED(g));
}
--
1.9.0.rc3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] bcache: fix BUG_ON due to integer overflow with GC_SECTORS_USED
2014-02-11 18:02 Darrick J. Wong
@ 2014-02-11 18:14 ` Greg KH
2014-02-11 18:18 ` Kent Overstreet
0 siblings, 1 reply; 6+ messages in thread
From: Greg KH @ 2014-02-11 18:14 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: stable, linux-bcache
On Tue, Feb 11, 2014 at 10:02:57AM -0800, Darrick J. Wong wrote:
> The BUG_ON at the end of __bch_btree_mark_key can be triggered due to
> an integer overflow error:
>
> BITMASK(GC_SECTORS_USED, struct bucket, gc_mark, 2, 13);
> ...
> SET_GC_SECTORS_USED(g, min_t(unsigned,
> GC_SECTORS_USED(g) + KEY_SIZE(k),
> (1 << 14) - 1));
> BUG_ON(!GC_SECTORS_USED(g));
>
> In bcache.h, the SECTORS_USED bitfield is defined to be 13 bits wide.
> While the SET_ code tries to ensure that the field doesn't overflow by
> clamping it to (1<<14)-1 == 16383, this is incorrect because 16383
> requires 14 bits. Therefore, if GC_SECTORS_USED() + KEY_SIZE() =
> 8192, the SET_ statement tries to store 8192 into a 13-bit field. In
> a 13-bit field, 8192 becomes zero, thus triggering the BUG_ON.
>
> Therefore, create a field width constant and a max value constant, and
> use those to create the bitfield and check the inputs to
> SET_GC_SECTORS_USED. Arguably the BITMASK() template ought to have
> BUG_ON checks for too-large values, but that's a separate patch.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> Signed-off-by: Kent Overstreet <kmo@daterainc.com>
> ---
> Greg, stable team et. all - can you apply this asap to 3.13? It hasn't hit
> Linus's tree yet (it's on its way via Jens), but multiple users are hitting this
> BUG_ON() and the fix has been tested.
Think about what you just wrote.
Then go read Documentation/stable_kernel_rules.txt.
You should know better...
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] bcache: fix BUG_ON due to integer overflow with GC_SECTORS_USED
2014-02-11 18:14 ` Greg KH
@ 2014-02-11 18:18 ` Kent Overstreet
2014-02-12 2:09 ` Darrick J. Wong
0 siblings, 1 reply; 6+ messages in thread
From: Kent Overstreet @ 2014-02-11 18:18 UTC (permalink / raw)
To: Greg KH; +Cc: stable, linux-bcache
On Tue, Feb 11, 2014 at 10:14:31AM -0800, Greg KH wrote:
> On Tue, Feb 11, 2014 at 10:02:57AM -0800, Darrick J. Wong wrote:
> > The BUG_ON at the end of __bch_btree_mark_key can be triggered due to
> > an integer overflow error:
> >
> > BITMASK(GC_SECTORS_USED, struct bucket, gc_mark, 2, 13);
> > ...
> > SET_GC_SECTORS_USED(g, min_t(unsigned,
> > GC_SECTORS_USED(g) + KEY_SIZE(k),
> > (1 << 14) - 1));
> > BUG_ON(!GC_SECTORS_USED(g));
> >
> > In bcache.h, the SECTORS_USED bitfield is defined to be 13 bits wide.
> > While the SET_ code tries to ensure that the field doesn't overflow by
> > clamping it to (1<<14)-1 == 16383, this is incorrect because 16383
> > requires 14 bits. Therefore, if GC_SECTORS_USED() + KEY_SIZE() =
> > 8192, the SET_ statement tries to store 8192 into a 13-bit field. In
> > a 13-bit field, 8192 becomes zero, thus triggering the BUG_ON.
> >
> > Therefore, create a field width constant and a max value constant, and
> > use those to create the bitfield and check the inputs to
> > SET_GC_SECTORS_USED. Arguably the BITMASK() template ought to have
> > BUG_ON checks for too-large values, but that's a separate patch.
> >
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > Signed-off-by: Kent Overstreet <kmo@daterainc.com>
> > ---
> > Greg, stable team et. all - can you apply this asap to 3.13? It hasn't hit
> > Linus's tree yet (it's on its way via Jens), but multiple users are hitting this
> > BUG_ON() and the fix has been tested.
>
> Think about what you just wrote.
>
> Then go read Documentation/stable_kernel_rules.txt.
>
> You should know better...
I figured if any patch qualified for skipping ahead this one would - and it
always takes awhile for stuff to make it through Jens, but ah well.
It's 947174476701fbc84ea8c7ec9664270f9d80b076, currently in Jens' for-linus
tree, I'll ping you again when I see it go in since I forgot to add the stable
tag to the patch.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] bcache: fix BUG_ON due to integer overflow with GC_SECTORS_USED
2014-02-11 18:18 ` Kent Overstreet
@ 2014-02-12 2:09 ` Darrick J. Wong
0 siblings, 0 replies; 6+ messages in thread
From: Darrick J. Wong @ 2014-02-12 2:09 UTC (permalink / raw)
To: Kent Overstreet; +Cc: Greg KH, stable, linux-bcache
On Tue, Feb 11, 2014 at 10:18:05AM -0800, Kent Overstreet wrote:
> On Tue, Feb 11, 2014 at 10:14:31AM -0800, Greg KH wrote:
> > On Tue, Feb 11, 2014 at 10:02:57AM -0800, Darrick J. Wong wrote:
> > > The BUG_ON at the end of __bch_btree_mark_key can be triggered due to
> > > an integer overflow error:
> > >
> > > BITMASK(GC_SECTORS_USED, struct bucket, gc_mark, 2, 13);
> > > ...
> > > SET_GC_SECTORS_USED(g, min_t(unsigned,
> > > GC_SECTORS_USED(g) + KEY_SIZE(k),
> > > (1 << 14) - 1));
> > > BUG_ON(!GC_SECTORS_USED(g));
> > >
> > > In bcache.h, the SECTORS_USED bitfield is defined to be 13 bits wide.
> > > While the SET_ code tries to ensure that the field doesn't overflow by
> > > clamping it to (1<<14)-1 == 16383, this is incorrect because 16383
> > > requires 14 bits. Therefore, if GC_SECTORS_USED() + KEY_SIZE() =
> > > 8192, the SET_ statement tries to store 8192 into a 13-bit field. In
> > > a 13-bit field, 8192 becomes zero, thus triggering the BUG_ON.
> > >
> > > Therefore, create a field width constant and a max value constant, and
> > > use those to create the bitfield and check the inputs to
> > > SET_GC_SECTORS_USED. Arguably the BITMASK() template ought to have
> > > BUG_ON checks for too-large values, but that's a separate patch.
> > >
> > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > > Signed-off-by: Kent Overstreet <kmo@daterainc.com>
> > > ---
> > > Greg, stable team et. all - can you apply this asap to 3.13? It hasn't hit
> > > Linus's tree yet (it's on its way via Jens), but multiple users are hitting this
> > > BUG_ON() and the fix has been tested.
> >
> > Think about what you just wrote.
> >
> > Then go read Documentation/stable_kernel_rules.txt.
> >
> > You should know better...
>
> I figured if any patch qualified for skipping ahead this one would - and it
> always takes awhile for stuff to make it through Jens, but ah well.
>
> It's 947174476701fbc84ea8c7ec9664270f9d80b076, currently in Jens' for-linus
> tree, I'll ping you again when I see it go in since I forgot to add the stable
> tag to the patch.
You might want to fix the From: header as well... ;)
--D
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bcache" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] bcache: fix BUG_ON due to integer overflow with GC_SECTORS_USED
@ 2014-01-29 0:57 Darrick J. Wong
0 siblings, 0 replies; 6+ messages in thread
From: Darrick J. Wong @ 2014-01-29 0:57 UTC (permalink / raw)
To: Kent Overstreet, Jose Manuel dos Santos Calhariz,
João Trindade
Cc: linux-bcache@vger.kernel.org
The BUG_ON at the end of __bch_btree_mark_key can be triggered due to
an integer overflow error:
BITMASK(GC_SECTORS_USED, struct bucket, gc_mark, 2, 13);
...
SET_GC_SECTORS_USED(g, min_t(unsigned,
GC_SECTORS_USED(g) + KEY_SIZE(k),
(1 << 14) - 1));
BUG_ON(!GC_SECTORS_USED(g));
In bcache.h, the SECTORS_USED bitfield is defined to be 13 bits wide.
While the SET_ code tries to ensure that the field doesn't overflow by
clamping it to (1<<14)-1 == 16383, this is incorrect because 16383
requires 14 bits. Therefore, if GC_SECTORS_USED() + KEY_SIZE() =
8192, the SET_ statement tries to store 8192 into a 13-bit field. In
a 13-bit field, 8192 becomes zero, thus triggering the BUG_ON.
Therefore, create a field width constant and a max value constant, and
use those to create the bitfield and check the inputs to
SET_GC_SECTORS_USED. Arguably the BITMASK() template ought to have
BUG_ON checks for too-large values, but that's a separate patch.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
drivers/md/bcache/bcache.h | 4 +++-
drivers/md/bcache/btree.c | 2 +-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/md/bcache/bcache.h b/drivers/md/bcache/bcache.h
index 754f431..cf3f6f8 100644
--- a/drivers/md/bcache/bcache.h
+++ b/drivers/md/bcache/bcache.h
@@ -209,7 +209,9 @@ BITMASK(GC_MARK, struct bucket, gc_mark, 0, 2);
#define GC_MARK_RECLAIMABLE 0
#define GC_MARK_DIRTY 1
#define GC_MARK_METADATA 2
-BITMASK(GC_SECTORS_USED, struct bucket, gc_mark, 2, 13);
+#define GC_SECTORS_USED_SIZE 13
+#define MAX_GC_SECTORS_USED (~(~0ULL << GC_SECTORS_USED_SIZE))
+BITMASK(GC_SECTORS_USED, struct bucket, gc_mark, 2, GC_SECTORS_USED_SIZE);
BITMASK(GC_MOVE, struct bucket, gc_mark, 15, 1);
#include "journal.h"
diff --git a/drivers/md/bcache/btree.c b/drivers/md/bcache/btree.c
index 31bb53f..7966ff8 100644
--- a/drivers/md/bcache/btree.c
+++ b/drivers/md/bcache/btree.c
@@ -1163,7 +1163,7 @@ uint8_t __bch_btree_mark_key(struct cache_set *c, int level, struct bkey *k)
/* guard against overflow */
SET_GC_SECTORS_USED(g, min_t(unsigned,
GC_SECTORS_USED(g) + KEY_SIZE(k),
- (1 << 14) - 1));
+ MAX_GC_SECTORS_USED));
BUG_ON(!GC_SECTORS_USED(g));
}
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-02-12 2:09 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-02-11 18:01 [PATCH] bcache: fix BUG_ON due to integer overflow with GC_SECTORS_USED Darrick J. Wong
-- strict thread matches above, loose matches on Subject: below --
2014-02-11 18:02 Darrick J. Wong
2014-02-11 18:14 ` Greg KH
2014-02-11 18:18 ` Kent Overstreet
2014-02-12 2:09 ` Darrick J. Wong
2014-01-29 0:57 Darrick J. Wong
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).