Commit 9d624aea authored by Coly Li's avatar Coly Li Committed by Kelsey Skunberg

bcache: fix super block seq numbers comparision in register_cache_set()

BugLink: https://bugs.launchpad.net/bugs/1892822

[ Upstream commit 117f636e ]

In register_cache_set(), c is pointer to struct cache_set, and ca is
pointer to struct cache, if ca->sb.seq > c->sb.seq, it means this
registering cache has up to date version and other members, the in-
memory version and other members should be updated to the newer value.

But current implementation makes a cache set only has a single cache
device, so the above assumption works well except for a special case.
The execption is when a cache device new created and both ca->sb.seq and
c->sb.seq are 0, because the super block is never flushed out yet. In
the location for the following if() check,
2156         if (ca->sb.seq > c->sb.seq) {
2157                 c->sb.version           = ca->sb.version;
2158                 memcpy(c->sb.set_uuid, ca->sb.set_uuid, 16);
2159                 c->sb.flags             = ca->sb.flags;
2160                 c->sb.seq               = ca->sb.seq;
2161                 pr_debug("set version = %llu\n", c->sb.version);
2162         }
c->sb.version is not initialized yet and valued 0. When ca->sb.seq is 0,
the if() check will fail (because both values are 0), and the cache set
version, set_uuid, flags and seq won't be updated.

The above problem is hiden for current code, because the bucket size is
compatible among different super block version. And the next time when
running cache set again, ca->sb.seq will be larger than 0 and cache set
super block version will be updated properly.

But if the large bucket feature is enabled,  sb->bucket_size is the low
16bits of the bucket size. For a power of 2 value, when the actual
bucket size exceeds 16bit width, sb->bucket_size will always be 0. Then
read_super_common() will fail because the if() check to
is_power_of_2(sb->bucket_size) is false. This is how the long time
hidden bug is triggered.

This patch modifies the if() check to the following way,
2156         if (ca->sb.seq > c->sb.seq || c->sb.seq == 0) {
Then cache set's version, set_uuid, flags and seq will always be updated
corectly including for a new created cache device.
Signed-off-by: default avatarColy Li <colyli@suse.de>
Reviewed-by: default avatarHannes Reinecke <hare@suse.de>
Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
Signed-off-by: default avatarSasha Levin <sashal@kernel.org>
Signed-off-by: default avatarKhalid Elmously <khalid.elmously@canonical.com>
Signed-off-by: default avatarIan May <ian.may@canonical.com>
Signed-off-by: default avatarKelsey Skunberg <kelsey.skunberg@canonical.com>
parent 8e58e272
...@@ -1815,7 +1815,14 @@ static const char *register_cache_set(struct cache *ca) ...@@ -1815,7 +1815,14 @@ static const char *register_cache_set(struct cache *ca)
sysfs_create_link(&c->kobj, &ca->kobj, buf)) sysfs_create_link(&c->kobj, &ca->kobj, buf))
goto err; goto err;
if (ca->sb.seq > c->sb.seq) { /*
* A special case is both ca->sb.seq and c->sb.seq are 0,
* such condition happens on a new created cache device whose
* super block is never flushed yet. In this case c->sb.version
* and other members should be updated too, otherwise we will
* have a mistaken super block version in cache set.
*/
if (ca->sb.seq > c->sb.seq || c->sb.seq == 0) {
c->sb.version = ca->sb.version; c->sb.version = ca->sb.version;
memcpy(c->sb.set_uuid, ca->sb.set_uuid, 16); memcpy(c->sb.set_uuid, ca->sb.set_uuid, 16);
c->sb.flags = ca->sb.flags; c->sb.flags = ca->sb.flags;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment