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@vger.kernel.org Signed-off-by: Joonwon Kang joonwonkang@google.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]]; }
/**