media: tegra: camera: sanity-check sizeofvalue ioctl parameter
(cherry pick from commit f793ae401d7a16623476b795e216d19e00cfe9d)
Several places in the camera stack can hit integer overflows or cause
bad allocations if userspace passes in a bogus sizeofvalue parameter.
Protect against this by using appropriately-sized integer types, adding
range checks, replacing array-allocation calls with kcalloc(), and
checking for allocations returning ZERO_SIZE_PTR.
For one specific ioctl (PCLLK_IOCTL_UPDATE) sizeofvalue = 0 is fine,
since when that happens the subdrivers won't actually touch the returned
allocation. In fact the existing userspace camera driver makes calls
like these and expects them to succeed! Handle this special case by
adding a __camera_get_params variant that optionally treats zero-sized
inputs as valid.
Reported-by: Jianqiang Zhao <zhaojianqiang1@gmail.com>
Signed-off-by: Greg Hackmann <ghackmann@google.com>
Bug: 27212204
Change-Id: Ie3250d8a4b814de5820fa0190b4cbd1af3ca4b3f
diff --git a/drivers/media/platform/tegra/cam_dev/imx135.c b/drivers/media/platform/tegra/cam_dev/imx135.c
index eaa0856..13613a4 100644
--- a/drivers/media/platform/tegra/cam_dev/imx135.c
+++ b/drivers/media/platform/tegra/cam_dev/imx135.c
@@ -42,11 +42,11 @@
};
static int imx135_update(
- struct camera_device *cdev, struct cam_update *upd, int num)
+ struct camera_device *cdev, struct cam_update *upd, u32 num)
{
/* struct imx135_info *info = dev_get_drvdata(cdev->dev); */
int err = 0;
- int idx;
+ u32 idx;
dev_dbg(cdev->dev, "%s %d\n", __func__, num);
mutex_lock(&cdev->mutex);
diff --git a/drivers/media/platform/tegra/cam_dev/of_camera.c b/drivers/media/platform/tegra/cam_dev/of_camera.c
index 460ca1a8..8b4518d 100644
--- a/drivers/media/platform/tegra/cam_dev/of_camera.c
+++ b/drivers/media/platform/tegra/cam_dev/of_camera.c
@@ -222,7 +222,7 @@
}
/* sanity check */
- if (!param.sizeofvalue) {
+ if (!param.sizeofvalue || param.sizeofvalue > INT_MAX) {
dev_err(cam->dev, "%s invalid property name length %d\n",
__func__, param.sizeofvalue);
return -EBADF;
diff --git a/drivers/media/platform/tegra/cam_dev/virtual.c b/drivers/media/platform/tegra/cam_dev/virtual.c
index 77fc955..e37392a 100644
--- a/drivers/media/platform/tegra/cam_dev/virtual.c
+++ b/drivers/media/platform/tegra/cam_dev/virtual.c
@@ -41,10 +41,10 @@
};
static int virtual_update(
- struct camera_device *cdev, struct cam_update *upd, int num)
+ struct camera_device *cdev, struct cam_update *upd, u32 num)
{
int err = 0;
- int idx;
+ u32 idx;
dev_dbg(cdev->dev, "%s %d\n", __func__, num);
mutex_lock(&cdev->mutex);
diff --git a/drivers/media/platform/tegra/camera.c b/drivers/media/platform/tegra/camera.c
index 7153eb7..5db3f2d 100644
--- a/drivers/media/platform/tegra/camera.c
+++ b/drivers/media/platform/tegra/camera.c
@@ -131,12 +131,12 @@
}
#endif
-int camera_get_params(
+int __camera_get_params(
struct camera_info *cam, unsigned long arg, int u_size,
- struct nvc_param *prm, void **data)
+ struct nvc_param *prm, void **data, bool zero_size_ok)
{
void *buf;
- unsigned size;
+ size_t size;
#ifdef CONFIG_COMPAT
memset(prm, 0, sizeof(*prm));
@@ -156,9 +156,14 @@
if (!data)
return 0;
+ if (zero_size_ok && prm->sizeofvalue == 0) {
+ *data = ZERO_SIZE_PTR;
+ return 0;
+ }
+
size = prm->sizeofvalue * u_size;
- buf = kzalloc(size, GFP_KERNEL);
- if (!buf) {
+ buf = kcalloc(prm->sizeofvalue, u_size, GFP_KERNEL);
+ if (ZERO_OR_NULL_PTR(buf)) {
dev_err(cam->dev, "%s allocate memory failed!\n", __func__);
return -ENOMEM;
}
@@ -231,7 +236,7 @@
}
p_i2c_table = devm_kzalloc(cdev->dev, params.sizeofvalue, GFP_KERNEL);
- if (p_i2c_table == NULL) {
+ if (ZERO_OR_NULL_PTR(p_i2c_table)) {
dev_err(cam->dev, "%s devm_kzalloc err line %d\n",
__func__, __LINE__);
return -ENOMEM;
@@ -586,7 +591,8 @@
return err;
}
- err = camera_get_params(cam, arg, sizeof(*upd), ¶m, (void **)&upd);
+ err = __camera_get_params(cam, arg, sizeof(*upd), ¶m, (void **)&upd,
+ true);
if (err)
return err;
diff --git a/include/media/camera.h b/include/media/camera.h
index fb2078b..73d2f16 100644
--- a/include/media/camera.h
+++ b/include/media/camera.h
@@ -338,7 +338,7 @@
int (*power_off)(struct camera_device *cdev);
int (*shutdown)(struct camera_device *cdev);
int (*update)(struct camera_device *cdev,
- struct cam_update *upd, int num);
+ struct cam_update *upd, u32 num);
};
struct camera_sync_dev {
@@ -387,8 +387,14 @@
};
/* common functions */
-int camera_get_params(
- struct camera_info *, unsigned long, int, struct nvc_param *, void **);
+int __camera_get_params(
+ struct camera_info *, unsigned long, int, struct nvc_param *, void **,
+ bool);
+static inline int camera_get_params(struct camera_info *cam, unsigned long arg,
+ int u_size, struct nvc_param *prm, void **data)
+{
+ return __camera_get_params(cam, arg, u_size, prm, data, false);
+}
int camera_copy_user_params(unsigned long, struct nvc_param *);
int virtual_device_add(struct device *, unsigned long);
