Five remoteproc/rpmsg fixes.
One was reported by Russell (the Kconfig fixes) and one by Omar (the endpoint
leak).
Ohad Ben-Cohen (5):
remoteproc: make sure we're parsing a 32bit firmware
remoteproc/omap: two Kconfig fixes
rpmsg: fix name service endpoint leak
rpmsg: validate incoming message length before propagating
rpmsg: fix published buffer length in rpmsg_recv_done
drivers/remoteproc/Kconfig | 3 +-
drivers/remoteproc/remoteproc_core.c | 8 ++++++
drivers/rpmsg/virtio_rpmsg_bus.c | 42 ++++++++++++++++++++++++++++-----
3 files changed, 44 insertions(+), 9 deletions(-)
--
1.7.5.4
Five trivial remoteproc/rpmsg fixes.
One was reported by Russell (the Kconfig fixes) and one by Omar (the endpoint
leak).
Ohad Ben-Cohen (5):
remoteproc: make sure we're parsing a 32bit firmware
remoteproc/omap: two Kconfig fixes
rpmsg: fix name service endpoint leak
rpmsg: validate incoming message length before propagating
rpmsg: fix published buffer length in rpmsg_recv_done
drivers/remoteproc/Kconfig | 3 +-
drivers/remoteproc/remoteproc_core.c | 8 ++++++
drivers/rpmsg/virtio_rpmsg_bus.c | 42 ++++++++++++++++++++++++++++-----
3 files changed, 44 insertions(+), 9 deletions(-)
--
1.7.5.4
1. Depend on OMAP_IOMMU instead of selecting it, to fix an unmet
direct dependency of it (and its imminent build error)
2. Set default to 'no' (achieved implicitly by dropping the 'default'
line)
Reported-by: Russell King <[email protected]>
Signed-off-by: Ohad Ben-Cohen <[email protected]>
Cc: Grant Likely <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Mark Grosen <[email protected]>
Cc: Suman Anna <[email protected]>
Cc: Fernando Guzman Lugo <[email protected]>
Cc: Rob Clark <[email protected]>
Cc: Ludovic BARRE <[email protected]>
Cc: Loic PALLARDY <[email protected]>
Cc: Omar Ramirez Luna <[email protected]>
Cc: Russell King <[email protected]>
---
drivers/remoteproc/Kconfig | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index 25fc4cc..24d880e 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -8,11 +8,10 @@ config REMOTEPROC
config OMAP_REMOTEPROC
tristate "OMAP remoteproc support"
depends on ARCH_OMAP4
- select OMAP_IOMMU
+ depends on OMAP_IOMMU
select REMOTEPROC
select OMAP_MBOX_FWK
select RPMSG
- default m
help
Say y here to support OMAP's remote processors (dual M3
and DSP on OMAP4) via the remote processor framework.
--
1.7.5.4
After processing an incoming message, always publish the real size
of its containing buffer when putting it back on the available rx ring.
Using any different value might erroneously limit the remote processor
(leading it to think the buffer is smaller than it really is).
Signed-off-by: Ohad Ben-Cohen <[email protected]>
Cc: Grant Likely <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Mark Grosen <[email protected]>
Cc: Suman Anna <[email protected]>
Cc: Fernando Guzman Lugo <[email protected]>
Cc: Rob Clark <[email protected]>
Cc: Ludovic BARRE <[email protected]>
Cc: Loic PALLARDY <[email protected]>
Cc: Omar Ramirez Luna <[email protected]>
---
drivers/rpmsg/virtio_rpmsg_bus.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c
index 1e8b8b6..f93ca3b 100644
--- a/drivers/rpmsg/virtio_rpmsg_bus.c
+++ b/drivers/rpmsg/virtio_rpmsg_bus.c
@@ -798,7 +798,8 @@ static void rpmsg_recv_done(struct virtqueue *rvq)
else
dev_warn(dev, "msg received with no recepient\n");
- sg_init_one(&sg, msg, sizeof(*msg) + len);
+ /* publish the real size of the buffer */
+ sg_init_one(&sg, msg, RPMSG_BUF_SIZE);
/* add the buffer back to the remote processor's virtqueue */
err = virtqueue_add_buf(vrp->rvq, &sg, 0, 1, msg, GFP_KERNEL);
--
1.7.5.4
The name service endpoint wasn't destroyed, so fix it.
This is achieved by introducing an internal __rpmsg_destroy_ept
function which doesn't assume the given ept is bound to an rpmsg
channel (much like the existing __rpmsg_create_ept).
This is needed because the name service ept belongs to the rpmsg bus,
and is never bound with a specific rpdev.
Reported-by: Omar Ramirez Luna <[email protected]>
Signed-off-by: Ohad Ben-Cohen <[email protected]>
Cc: Grant Likely <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Mark Grosen <[email protected]>
Cc: Suman Anna <[email protected]>
Cc: Fernando Guzman Lugo <[email protected]>
Cc: Rob Clark <[email protected]>
Cc: Ludovic BARRE <[email protected]>
Cc: Loic PALLARDY <[email protected]>
Cc: Omar Ramirez Luna <[email protected]>
---
drivers/rpmsg/virtio_rpmsg_bus.c | 29 +++++++++++++++++++++++------
1 files changed, 23 insertions(+), 6 deletions(-)
diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c
index 8980ac2..4db9cf8 100644
--- a/drivers/rpmsg/virtio_rpmsg_bus.c
+++ b/drivers/rpmsg/virtio_rpmsg_bus.c
@@ -290,22 +290,36 @@ struct rpmsg_endpoint *rpmsg_create_ept(struct rpmsg_channel *rpdev,
EXPORT_SYMBOL(rpmsg_create_ept);
/**
- * rpmsg_destroy_ept() - destroy an existing rpmsg endpoint
+ * __rpmsg_destroy_ept() - destroy an existing rpmsg endpoint
+ * @vrp: virtproc which owns this ept
* @ept: endpoing to destroy
*
- * Should be used by drivers to destroy an rpmsg endpoint previously
- * created with rpmsg_create_ept().
+ * An internal function which destroy an ept without assuming it is
+ * bound to an rpmsg channel. This is needed for handling the internal
+ * name service endpoint, which isn't bound to an rpmsg channel.
+ * See also __rpmsg_create_ept().
*/
-void rpmsg_destroy_ept(struct rpmsg_endpoint *ept)
+static void
+__rpmsg_destroy_ept(struct virtproc_info *vrp, struct rpmsg_endpoint *ept)
{
- struct virtproc_info *vrp = ept->rpdev->vrp;
-
mutex_lock(&vrp->endpoints_lock);
idr_remove(&vrp->endpoints, ept->addr);
mutex_unlock(&vrp->endpoints_lock);
kfree(ept);
}
+
+/**
+ * rpmsg_destroy_ept() - destroy an existing rpmsg endpoint
+ * @ept: endpoing to destroy
+ *
+ * Should be used by drivers to destroy an rpmsg endpoint previously
+ * created with rpmsg_create_ept().
+ */
+void rpmsg_destroy_ept(struct rpmsg_endpoint *ept)
+{
+ __rpmsg_destroy_ept(ept->rpdev->vrp, ept);
+}
EXPORT_SYMBOL(rpmsg_destroy_ept);
/*
@@ -964,6 +978,9 @@ static void __devexit rpmsg_remove(struct virtio_device *vdev)
if (ret)
dev_warn(&vdev->dev, "can't remove rpmsg device: %d\n", ret);
+ if (vrp->ns_ept)
+ __rpmsg_destroy_ept(vrp, vrp->ns_ept);
+
idr_remove_all(&vrp->endpoints);
idr_destroy(&vrp->endpoints);
--
1.7.5.4
When an inbound message arrives, validate its reported length before
propagating it, otherwise buggy (or malicious) remote processors might
trick us into accessing memory which we really shouldn't.
Signed-off-by: Ohad Ben-Cohen <[email protected]>
Cc: Grant Likely <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Mark Grosen <[email protected]>
Cc: Suman Anna <[email protected]>
Cc: Fernando Guzman Lugo <[email protected]>
Cc: Rob Clark <[email protected]>
Cc: Ludovic BARRE <[email protected]>
Cc: Loic PALLARDY <[email protected]>
Cc: Omar Ramirez Luna <[email protected]>
---
drivers/rpmsg/virtio_rpmsg_bus.c | 10 ++++++++++
1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c
index 4db9cf8..1e8b8b6 100644
--- a/drivers/rpmsg/virtio_rpmsg_bus.c
+++ b/drivers/rpmsg/virtio_rpmsg_bus.c
@@ -778,6 +778,16 @@ static void rpmsg_recv_done(struct virtqueue *rvq)
print_hex_dump(KERN_DEBUG, "rpmsg_virtio RX: ", DUMP_PREFIX_NONE, 16, 1,
msg, sizeof(*msg) + msg->len, true);
+ /*
+ * We currently use fixed-sized buffers, so trivially sanitize
+ * the reported payload length.
+ */
+ if (len > RPMSG_BUF_SIZE ||
+ msg->len > (len - sizeof(struct rpmsg_hdr))) {
+ dev_warn(dev, "inbound msg too big: (%d, %d)\n", len, msg->len);
+ return;
+ }
+
/* use the dst addr to fetch the callback of the appropriate user */
mutex_lock(&vrp->endpoints_lock);
ept = idr_find(&vrp->endpoints, msg->dst);
--
1.7.5.4
Make sure we're parsing a 32bit image, since we only support
the ELF32 binary format at this point.
This should prevent unexpected behavior with non 32bit binaries.
Signed-off-by: Ohad Ben-Cohen <[email protected]>
Cc: Grant Likely <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Mark Grosen <[email protected]>
Cc: Suman Anna <[email protected]>
Cc: Fernando Guzman Lugo <[email protected]>
Cc: Rob Clark <[email protected]>
Cc: Ludovic BARRE <[email protected]>
Cc: Loic PALLARDY <[email protected]>
Cc: Omar Ramirez Luna <[email protected]>
---
drivers/remoteproc/remoteproc_core.c | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 729911b..8990c51 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -840,6 +840,7 @@ static int rproc_fw_sanity_check(struct rproc *rproc, const struct firmware *fw)
const char *name = rproc->firmware;
struct device *dev = rproc->dev;
struct elf32_hdr *ehdr;
+ char class;
if (!fw) {
dev_err(dev, "failed to load %s\n", name);
@@ -853,6 +854,13 @@ static int rproc_fw_sanity_check(struct rproc *rproc, const struct firmware *fw)
ehdr = (struct elf32_hdr *)fw->data;
+ /* We only support ELF32 at this point */
+ class = ehdr->e_ident[EI_CLASS];
+ if (class != ELFCLASS32) {
+ dev_err(dev, "Unsupported class: %d\n", class);
+ return -EINVAL;
+ }
+
/* We assume the firmware has the same endianess as the host */
# ifdef __LITTLE_ENDIAN
if (ehdr->e_ident[EI_DATA] != ELFDATA2LSB) {
--
1.7.5.4