[PATCH v4] mailbox: Prevent out-of-bounds access in fw_mbox_index_xlate()

[Joonwon Kang <joonwonkang@google.com>]

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]];
 }
 
 /**
-- 
2.52.0.487.g5c8c507ade-goog