From: Kent Overstreet <kent.overstreet@linux.dev>
To: Camila Alvarez <cam.alvarez.i@gmail.com>
Cc: Brian Foster <bfoster@redhat.com>,
linux-bcachefs@vger.kernel.org, linux-kernel@vger.kernel.org,
syzbot+249018ea545364f78d04@syzkaller.appspotmail.com
Subject: Re: [PATCH] guard against unknown r->data_type in bch2_sb_dev_has_data()
Date: Sun, 19 May 2024 19:48:47 -0400 [thread overview]
Message-ID: <463goq53266oiymf2bpulp2jczyd37boxifqhem3hhhu5vxn3z@xdjdgmx3htsq> (raw)
In-Reply-To: <20240515014720.2482142-1-cam.alvarez.i@gmail.com>
On Tue, May 14, 2024 at 09:47:22PM -0400, Camila Alvarez wrote:
> Shift is perfomed only when the data type makes sense.
>
> Fix a missing guard on r->data_type being known.
>
> Reported-by: syzbot+249018ea545364f78d04@syzkaller.appspotmail.com
> Signed-off-by: Camila Alvarez <cam.alvarez.i@gmail.com>
Sorry I just now saw this, already applied a different fix - BCH_DATA_NR
isn't the right guard here, because we need to be able to handle unknown
data types for backwards compatibilicy.
Here's my version:
commit 85e8b546542c9a1f9050a7f53d92b8bd39c8e9d8
Author: Kent Overstreet <kent.overstreet@linux.dev>
Date: Sun May 19 17:28:44 2024 -0400
bcachefs: Fix shift overflows in replicas.c
We can't disallow unknown data_types in verify() - we have to preserve
them unchanged for backwards compat; that means we have to add a few
more guards.
Reported-by: syzbot+249018ea545364f78d04@syzkaller.appspotmail.com
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
diff --git a/fs/bcachefs/replicas.c b/fs/bcachefs/replicas.c
index bd1d5d085e23..d45b10bc531c 100644
--- a/fs/bcachefs/replicas.c
+++ b/fs/bcachefs/replicas.c
@@ -524,13 +524,16 @@ int bch2_replicas_gc_start(struct bch_fs *c, unsigned typemask)
c->replicas_gc.nr = 0;
c->replicas_gc.entry_size = 0;
- for_each_cpu_replicas_entry(&c->replicas, e)
- if (!((1 << e->data_type) & typemask)) {
+ for_each_cpu_replicas_entry(&c->replicas, e) {
+ /* Preserve unknown data types */
+ if (e->data_type >= BCH_DATA_NR ||
+ !((1 << e->data_type) & typemask)) {
c->replicas_gc.nr++;
c->replicas_gc.entry_size =
max_t(unsigned, c->replicas_gc.entry_size,
replicas_entry_bytes(e));
}
+ }
c->replicas_gc.entries = kcalloc(c->replicas_gc.nr,
c->replicas_gc.entry_size,
@@ -542,7 +545,8 @@ int bch2_replicas_gc_start(struct bch_fs *c, unsigned typemask)
}
for_each_cpu_replicas_entry(&c->replicas, e)
- if (!((1 << e->data_type) & typemask))
+ if (e->data_type >= BCH_DATA_NR ||
+ !((1 << e->data_type) & typemask))
memcpy(cpu_replicas_entry(&c->replicas_gc, i++),
e, c->replicas_gc.entry_size);
@@ -998,7 +1002,7 @@ unsigned bch2_sb_dev_has_data(struct bch_sb *sb, unsigned dev)
{
struct bch_sb_field_replicas *replicas;
struct bch_sb_field_replicas_v0 *replicas_v0;
- unsigned i, data_has = 0;
+ unsigned data_has = 0;
replicas = bch2_sb_field_get(sb, replicas);
replicas_v0 = bch2_sb_field_get(sb, replicas_v0);
@@ -1006,17 +1010,26 @@ unsigned bch2_sb_dev_has_data(struct bch_sb *sb, unsigned dev)
if (replicas) {
struct bch_replicas_entry_v1 *r;
- for_each_replicas_entry(replicas, r)
- for (i = 0; i < r->nr_devs; i++)
+ for_each_replicas_entry(replicas, r) {
+ if (r->data_type >= sizeof(data_has) * 8)
+ continue;
+
+ for (unsigned i = 0; i < r->nr_devs; i++)
if (r->devs[i] == dev)
data_has |= 1 << r->data_type;
+ }
+
} else if (replicas_v0) {
struct bch_replicas_entry_v0 *r;
- for_each_replicas_entry_v0(replicas_v0, r)
- for (i = 0; i < r->nr_devs; i++)
+ for_each_replicas_entry_v0(replicas_v0, r) {
+ if (r->data_type >= sizeof(data_has) * 8)
+ continue;
+
+ for (unsigned i = 0; i < r->nr_devs; i++)
if (r->devs[i] == dev)
data_has |= 1 << r->data_type;
+ }
}
prev parent reply other threads:[~2024-05-19 23:48 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-15 1:47 [PATCH] guard against unknown r->data_type in bch2_sb_dev_has_data() Camila Alvarez
2024-05-19 23:48 ` Kent Overstreet [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=463goq53266oiymf2bpulp2jczyd37boxifqhem3hhhu5vxn3z@xdjdgmx3htsq \
--to=kent.overstreet@linux.dev \
--cc=bfoster@redhat.com \
--cc=cam.alvarez.i@gmail.com \
--cc=linux-bcachefs@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=syzbot+249018ea545364f78d04@syzkaller.appspotmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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 read-only IMAP folder(s) and NNTP newsgroup(s).