[<prev] [next>] [day] [month] [year] [list]
Message-ID: <20251126062250.2566655-1-joonwonkang@google.com>
Date: Wed, 26 Nov 2025 06:22:50 +0000
From: Joonwon Kang <joonwonkang@...gle.com>
To: jassisinghbrar@...il.com, broonie@...nel.org
Cc: peng.fan@....nxp.com, linux-kernel@...r.kernel.org, stable@...r.kernel.org,
security@...nel.org, Joonwon Kang <joonwonkang@...gle.com>
Subject: [PATCH v4] mailbox: Prevent out-of-bounds access in fw_mbox_index_xlate()
Although it is guided that `#mbox-cells` must be at least 1, there are
many instances of `#mbox-cells = <0>;` in the device tree. If that is
the case and the corresponding mailbox controller does not provide
`fw_xlate` and of_xlate` function pointers, `fw_mbox_index_xlate()` will
be used by default and out-of-bounds accesses could occur due to lack of
bounds check in that function.
Cc: stable@...r.kernel.org
Signed-off-by: Joonwon Kang <joonwonkang@...gle.com>
---
V3 -> V4: Prevented access to sp->args[0] if sp->nargs < 1 and rebased
on the linux-next tree.
For CVE review, below is a problematic control flow when
`#mbox-cells = <0>;`:
```
static struct mbox_chan *
fw_mbox_index_xlate(struct mbox_controller *mbox,
const struct fwnode_reference_args *sp)
{
int ind = sp->args[0]; // (4)
if (ind >= mbox->num_chans) // (5)
return ERR_PTR(-EINVAL);
return &mbox->chans[ind]; // (6)
}
struct mbox_chan *mbox_request_channel(struct mbox_client *cl, int index)
{
...
struct fwnode_reference_args fwspec; // (1)
...
ret = fwnode_property_get_reference_args(fwnode, "mboxes", // (2)
"#mbox-cells", 0, index, &fwspec);
...
scoped_guard(mutex, &con_mutex) {
...
list_for_each_entry(mbox, &mbox_cons, node) {
if (device_match_fwnode(mbox->dev, fwspec.fwnode)) {
if (mbox->fw_xlate) {
chan = mbox->fw_xlate(mbox, &fwspec); // (3)
if (!IS_ERR(chan))
break;
}
...
}
}
...
ret = __mbox_bind_client(chan, cl); // (7)
...
}
...
}
static int __mbox_bind_client(struct mbox_chan *chan,
struct mbox_client *cl)
{
if (chan->cl || ...) { // (8)
}
```
(1) `fwspec.args[]` is filled with arbitrary leftover values in the stack.
Let's say that `fwspec.args[0] == 0xffffffff`.
(2) Since `#mbox-cells = <0>;`, `fwspec.nargs` is assigned 0 and
`fwspec.args[]` are untouched.
(3) Since the controller has not provided `fw_xlate` and `of_xlate`,
`fw_mbox_index_xlate()` is used instead.
(4) `idx` is assigned -1 due to the value of `fwspec.args[0]`.
(5) Since `mbox->num_chans >= 0` and `idx == -1`, this condition does
not filter out this case.
(6) Out-of-bounds address is returned. Depending on what was left in
`fwspec.args[0]`, it could be an arbitrary(but confined to a specific
range) address.
(7) A function is called with the out-of-bounds address in `chan`.
(8) The out-of-bounds address is accessed.
drivers/mailbox/mailbox.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
index 2acc6ec229a4..617ba505691d 100644
--- a/drivers/mailbox/mailbox.c
+++ b/drivers/mailbox/mailbox.c
@@ -489,12 +489,10 @@ EXPORT_SYMBOL_GPL(mbox_free_channel);
static struct mbox_chan *fw_mbox_index_xlate(struct mbox_controller *mbox,
const struct fwnode_reference_args *sp)
{
- int ind = sp->args[0];
-
- if (ind >= mbox->num_chans)
+ if (sp->nargs < 1 || sp->args[0] >= mbox->num_chans)
return ERR_PTR(-EINVAL);
- return &mbox->chans[ind];
+ return &mbox->chans[sp->args[0]];
}
/**
--
2.52.0.487.g5c8c507ade-goog
Powered by blists - more mailing lists