@@ -55,6 +55,8 @@ static int media_device_open(struct media_devnode *devnode, struct file *filp)
if (!fh)
return -ENOMEM;
+ fh->fh.ref = devnode->ref;
+
filp->private_data = &fh->fh;
spin_lock_irqsave(&mdev->fh_list_lock, flags);
@@ -66,14 +68,19 @@ static int media_device_open(struct media_devnode *devnode, struct file *filp)
static int media_device_close(struct file *filp)
{
- struct media_devnode *devnode = media_devnode_data(filp);
- struct media_device *mdev = to_media_device(devnode);
- struct media_device_fh *fh = media_device_fh(filp);
- unsigned long flags;
+ struct media_device_fh *fh;
- spin_lock_irqsave(&mdev->fh_list_lock, flags);
- list_del(&fh->mdev_list);
- spin_unlock_irqrestore(&mdev->fh_list_lock, flags);
+ fh = media_device_fh(filp);
+
+ if (!fh->fh.ref || atomic_read(&fh->fh.ref->registered)) {
+ struct media_devnode *devnode = media_devnode_data(filp);
+ struct media_device *mdev = to_media_device(devnode);
+ unsigned long flags;
+
+ spin_lock_irqsave(&mdev->fh_list_lock, flags);
+ list_del(&fh->mdev_list);
+ spin_unlock_irqrestore(&mdev->fh_list_lock, flags);
+ }
kfree(fh);
@@ -812,28 +819,45 @@ EXPORT_SYMBOL_GPL(media_device_cleanup);
int __must_check __media_device_register(struct media_device *mdev,
struct module *owner)
{
+ struct media_devnode_compat_ref *ref = NULL;
int ret;
+ if (!mdev->ops || !mdev->ops->release) {
+ ref = kzalloc(sizeof(*mdev->devnode.ref), GFP_KERNEL);
+ if (!ref)
+ return -ENOMEM;
+ }
+
/* Register the device node. */
mdev->devnode.fops = &media_device_fops;
mdev->devnode.parent = mdev->dev;
+ mdev->devnode.ref = ref;
/* Set version 0 to indicate user-space that the graph is static */
mdev->topology_version = 0;
ret = media_devnode_register(&mdev->devnode, owner);
if (ret < 0)
- return ret;
+ goto out_put_ref;
+
+ ret = device_create_file(media_devnode_dev(&mdev->devnode),
+ &dev_attr_model);
+ if (ret < 0)
+ goto out_devnode_unregister;
- ret = device_create_file(&mdev->devnode.dev, &dev_attr_model);
- if (ret < 0) {
- media_devnode_unregister(&mdev->devnode);
- return ret;
- }
dev_dbg(mdev->dev, "Media device registered\n");
return 0;
+
+out_devnode_unregister:
+ media_devnode_unregister(&mdev->devnode);
+
+out_put_ref:
+ if (ref)
+ put_device(&ref->dev);
+
+ return ret;
}
EXPORT_SYMBOL_GPL(__media_device_register);
@@ -855,9 +879,12 @@ void media_device_unregister(struct media_device *mdev)
list_del_init(&mdev->fh_list);
spin_unlock_irqrestore(&mdev->fh_list_lock, flags);
- device_remove_file(&mdev->devnode.dev, &dev_attr_model);
+ device_remove_file(media_devnode_dev(&mdev->devnode), &dev_attr_model);
dev_dbg(mdev->dev, "Media device unregistering\n");
media_devnode_unregister(&mdev->devnode);
+
+ if (mdev->devnode.ref)
+ put_device(&mdev->devnode.ref->dev);
}
EXPORT_SYMBOL_GPL(media_device_unregister);
@@ -71,19 +71,45 @@ static void media_devnode_release(struct device *cd)
devnode->release(devnode);
}
+static void media_devnode_ref_release(struct device *cd)
+{
+ struct media_devnode_compat_ref *ref =
+ container_of_const(cd, struct media_devnode_compat_ref, dev);
+
+ media_devnode_free_minor(ref->minor);
+
+ kfree(ref);
+}
+
+struct media_devnode *to_media_devnode(struct device *dev)
+{
+ if (dev->release == media_devnode_release)
+ return container_of(dev, struct media_devnode, dev);
+
+ return container_of(dev, struct media_devnode_compat_ref, dev)->devnode;
+}
+
static struct bus_type media_bus_type = {
.name = MEDIA_NAME,
};
+static bool media_devnode_is_registered_compat(struct media_devnode_fh *fh)
+{
+ if (fh->ref)
+ return atomic_read(&fh->ref->registered);
+
+ return media_devnode_is_registered(fh->devnode);
+}
+
static ssize_t media_read(struct file *filp, char __user *buf,
size_t sz, loff_t *off)
{
struct media_devnode *devnode = media_devnode_data(filp);
+ if (!media_devnode_is_registered_compat(filp->private_data))
+ return -EIO;
if (!devnode->fops->read)
return -EINVAL;
- if (!media_devnode_is_registered(devnode))
- return -EIO;
return devnode->fops->read(filp, buf, sz, off);
}
@@ -92,10 +118,10 @@ static ssize_t media_write(struct file *filp, const char __user *buf,
{
struct media_devnode *devnode = media_devnode_data(filp);
+ if (!media_devnode_is_registered_compat(filp->private_data))
+ return -EIO;
if (!devnode->fops->write)
return -EINVAL;
- if (!media_devnode_is_registered(devnode))
- return -EIO;
return devnode->fops->write(filp, buf, sz, off);
}
@@ -104,7 +130,7 @@ static __poll_t media_poll(struct file *filp,
{
struct media_devnode *devnode = media_devnode_data(filp);
- if (!media_devnode_is_registered(devnode))
+ if (!media_devnode_is_registered_compat(filp->private_data))
return EPOLLERR | EPOLLHUP;
if (!devnode->fops->poll)
return DEFAULT_POLLMASK;
@@ -116,14 +142,9 @@ __media_ioctl(struct file *filp, unsigned int cmd, unsigned long arg,
long (*ioctl_func)(struct file *filp, unsigned int cmd,
unsigned long arg))
{
- struct media_devnode *devnode = media_devnode_data(filp);
-
if (!ioctl_func)
return -ENOTTY;
- if (!media_devnode_is_registered(devnode))
- return -EIO;
-
return ioctl_func(filp, cmd, arg);
}
@@ -131,6 +152,9 @@ static long media_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
{
struct media_devnode *devnode = media_devnode_data(filp);
+ if (!media_devnode_is_registered_compat(filp->private_data))
+ return -EIO;
+
return __media_ioctl(filp, cmd, arg, devnode->fops->ioctl);
}
@@ -141,6 +165,9 @@ static long media_compat_ioctl(struct file *filp, unsigned int cmd,
{
struct media_devnode *devnode = media_devnode_data(filp);
+ if (!media_devnode_is_registered_compat(filp->private_data))
+ return -EIO;
+
return __media_ioctl(filp, cmd, arg, devnode->fops->compat_ioctl);
}
@@ -149,6 +176,7 @@ static long media_compat_ioctl(struct file *filp, unsigned int cmd,
/* Override for the open function */
static int media_open(struct inode *inode, struct file *filp)
{
+ struct media_devnode_cdev *mcdev;
struct media_devnode *devnode;
struct media_devnode_fh *fh;
int ret;
@@ -160,7 +188,12 @@ static int media_open(struct inode *inode, struct file *filp)
* a crash.
*/
mutex_lock(&media_devnode_lock);
- devnode = container_of(inode->i_cdev, struct media_devnode, cdev);
+ mcdev = container_of(inode->i_cdev, struct media_devnode_cdev, cdev);
+ if (mcdev->is_compat_ref)
+ devnode = container_of(mcdev, struct media_devnode_compat_ref,
+ mcdev)->devnode;
+ else
+ devnode = container_of(mcdev, struct media_devnode, mcdev);
/* return ENXIO if the media device has been removed
already or if it is not registered anymore. */
if (!media_devnode_is_registered(devnode)) {
@@ -168,12 +201,12 @@ static int media_open(struct inode *inode, struct file *filp)
return -ENXIO;
}
/* and increase the device refcount */
- get_device(&devnode->dev);
+ get_device(media_devnode_dev(devnode));
mutex_unlock(&media_devnode_lock);
ret = devnode->fops->open(devnode, filp);
if (ret) {
- put_device(&devnode->dev);
+ put_device(media_devnode_dev(devnode));
return ret;
}
@@ -186,15 +219,21 @@ static int media_open(struct inode *inode, struct file *filp)
/* Override for the release function */
static int media_release(struct inode *inode, struct file *filp)
{
- struct media_devnode *devnode = media_devnode_data(filp);
-
- devnode->fops->release(filp);
+ struct media_devnode_fh *fh = filp->private_data;
+ struct device *dev;
+
+ if (!fh->ref) {
+ dev = &fh->devnode->dev;
+ fh->devnode->fops->release(filp);
+ } else {
+ dev = &fh->ref->dev;
+ fh->ref->release(filp);
+ }
filp->private_data = NULL;
- /* decrease the refcount unconditionally since the release()
- return value is ignored. */
- put_device(&devnode->dev);
+ put_device(dev);
+
return 0;
}
@@ -222,6 +261,9 @@ void media_devnode_init(struct media_devnode *devnode)
int __must_check media_devnode_register(struct media_devnode *devnode,
struct module *owner)
{
+ struct media_devnode_compat_ref *ref = devnode->ref;
+ struct cdev *cdev;
+ struct device *dev;
int minor;
int ret;
@@ -243,18 +285,31 @@ int __must_check media_devnode_register(struct media_devnode *devnode,
devnode->minor = minor;
/* Part 2: Initialize the media and character devices */
- cdev_init(&devnode->cdev, &media_devnode_fops);
- devnode->cdev.owner = owner;
- kobject_set_name(&devnode->cdev.kobj, "media%d", devnode->minor);
-
- devnode->dev.bus = &media_bus_type;
- devnode->dev.devt = MKDEV(MAJOR(media_dev_t), devnode->minor);
+ cdev = ref ? &ref->mcdev.cdev : &devnode->mcdev.cdev;
+ cdev_init(cdev, &media_devnode_fops);
+ cdev->owner = owner;
+ kobject_set_name(&cdev->kobj, "media%d", devnode->minor);
+
+ if (!ref) {
+ dev = &devnode->dev;
+ } else {
+ ref->mcdev.is_compat_ref = true;
+ device_initialize(&ref->dev);
+ atomic_set(&ref->registered, 1);
+ ref->devnode = devnode;
+ ref->minor = devnode->minor;
+ ref->release = devnode->fops->release;
+ dev = &ref->dev;
+ dev->release = media_devnode_ref_release;
+ }
+ dev->bus = &media_bus_type;
+ dev->devt = MKDEV(MAJOR(media_dev_t), devnode->minor);
if (devnode->parent)
- devnode->dev.parent = devnode->parent;
- dev_set_name(&devnode->dev, "media%d", devnode->minor);
+ dev->parent = devnode->parent;
+ dev_set_name(dev, "media%d", devnode->minor);
/* Part 3: Add the media and character devices */
- ret = cdev_device_add(&devnode->cdev, &devnode->dev);
+ ret = cdev_device_add(cdev, dev);
if (ret < 0) {
pr_err("%s: cdev_device_add failed\n", __func__);
goto cdev_add_error;
@@ -279,11 +334,15 @@ void media_devnode_unregister(struct media_devnode *devnode)
if (!media_devnode_is_registered(devnode))
return;
+ if (devnode->ref)
+ atomic_set(&devnode->ref->registered, 0);
+
mutex_lock(&media_devnode_lock);
clear_bit(MEDIA_FLAG_REGISTERED, &devnode->flags);
mutex_unlock(&media_devnode_lock);
- cdev_device_del(&devnode->cdev, &devnode->dev);
+ cdev_device_del(devnode->ref ? &devnode->ref->mcdev.cdev :
+ &devnode->mcdev.cdev, media_devnode_dev(devnode));
}
/*
@@ -179,7 +179,7 @@ static void v4l2_device_release(struct device *cd)
bool v4l2_dev_has_release = v4l2_dev->release;
#ifdef CONFIG_MEDIA_CONTROLLER
struct media_device *mdev = v4l2_dev->mdev;
- bool mdev_has_release = mdev && mdev->ops && mdev->ops->release;
+ bool mdev_has_ref = mdev && mdev->devnode.ref;
#endif
mutex_lock(&videodev_lock);
@@ -212,12 +212,24 @@ static void v4l2_device_release(struct device *cd)
}
#endif
- /* Release video_device and perform other
- cleanups as needed. */
+ /*
+ * Put struct media_devnode_compat_ref here as indicated by
+ * mdev_has_ref. mdev may be released by vdev->release() below.
+ */
+#ifdef CONFIG_MEDIA_CONTROLLER
+ if (mdev && mdev_has_ref)
+ media_device_put(mdev);
+#endif
+
+ /* Release video_device and perform other cleanups as needed. */
vdev->release(vdev);
#ifdef CONFIG_MEDIA_CONTROLLER
- if (mdev)
+ /*
+ * Put a reference to struct media_device acquired in
+ * video_register_media_controller().
+ */
+ if (mdev && !mdev_has_ref)
media_device_put(mdev);
/*
@@ -225,16 +237,18 @@ static void v4l2_device_release(struct device *cd)
* embedded in the same driver's context struct so having a release
* callback in both is a bug.
*/
- WARN_ON(v4l2_dev_has_release && mdev_has_release);
+ WARN_ON(v4l2_dev_has_release && !mdev_has_ref);
#endif
/*
* Decrease v4l2_device refcount, but only if the media device doesn't
- * have a release callback.
+ * have a release callback. Otherwise one could expect accessing
+ * released memory --- driver's context struct refcounted already via
+ * struct media_device.
*/
if (v4l2_dev_has_release
#ifdef CONFIG_MEDIA_CONTROLLER
- && !mdev_has_release
+ && mdev_has_ref
#endif
)
v4l2_device_put(v4l2_dev);
@@ -41,8 +41,6 @@ struct media_devnode;
* @compat_ioctl: pointer to the function that will handle 32 bits userspace
* calls to the ioctl() syscall on a Kernel compiled with 64 bits.
* @open: pointer to the function that implements open() syscall
- * @release: pointer to the function that will release the resources allocated
- * by the @open function.
*/
struct media_file_operations {
struct module *owner;
@@ -55,9 +53,56 @@ struct media_file_operations {
int (*release) (struct file *);
};
+/**
+ * struct media_devnode_cdev - Workaround for drivers not managing media device
+ * lifetime - character device
+ *
+ * Store the characeter device and whether this is a compatibility reference or
+ * not. struct media_devnode_cdev is embedded in either struct
+ * media_devnode_compat_ref or struct media_devnode.
+ *
+ * @cdev: the chracter device
+ * @is_compat_ref: Is this a compatibility reference or not?
+ */
+struct media_devnode_cdev {
+ struct cdev cdev;
+ bool is_compat_ref;
+};
+
+/**
+ * struct media_devnode_compat_ref - Workaround for drivers not managing media
+ * device lifetime - reference
+ *
+ * The purpose if this struct is to support drivers that do not manage the
+ * lifetime of their respective media devices to avoid the worst effects of
+ * this, namely an IOCTL call on an open file handle to a device that has been
+ * unbound causing a kernel oops systematically. This is not a fix. The proper,
+ * reliable way to handle this is to manage the resources used by the
+ * driver. This struct and its use can be removed once all drivers have been
+ * converted.
+ *
+ * @dev: struct device that remains in place as long as any reference remains
+ * @cdev: the related character device
+ * @devnode: a pointer back to the devnode
+ * @release: pointer to the function that will release the resources
+ * allocated by the @open function.
+ * @registered: is this device registered?
+ * @minor: the minor number of the media devnode
+ */
+struct media_devnode_compat_ref {
+ struct device dev;
+ struct media_devnode_cdev mcdev;
+ struct media_devnode *devnode;
+ int (*release)(struct file *);
+ atomic_t registered;
+ unsigned int minor;
+};
+
/**
* struct media_devnode_fh - Media device node file handle
* @devnode: pointer to the media device node
+ * @ref: media device compat ref, if the driver does not manage media
+ * device lifetime
*
* This structure serves as a base for per-file-handle data storage. Media
* device node users embed media_devnode_fh in their custom file handle data
@@ -67,6 +112,7 @@ struct media_file_operations {
*/
struct media_devnode_fh {
struct media_devnode *devnode;
+ struct media_devnode_compat_ref *ref;
};
/**
@@ -80,6 +126,8 @@ struct media_devnode_fh {
* @flags: flags, combination of the ``MEDIA_FLAG_*`` constants
* @release: release callback called at the end of ``media_devnode_release()``
* routine at media-device.c.
+ * @ref: reference for providing best effort memory safety in device
+ * removal
*
* This structure represents a media-related device node.
*
@@ -92,7 +140,7 @@ struct media_devnode {
/* sysfs */
struct device dev; /* media device */
- struct cdev cdev; /* character device */
+ struct media_devnode_cdev mcdev; /* character device + compat status */
struct device *parent; /* device parent */
/* device info */
@@ -101,10 +149,18 @@ struct media_devnode {
/* callbacks */
void (*release)(struct media_devnode *devnode);
+
+ /* compat reference */
+ struct media_devnode_compat_ref *ref;
};
+static inline struct device *media_devnode_dev(struct media_devnode *devnode)
+{
+ return devnode->ref ? &devnode->ref->dev : &devnode->dev;
+}
+
/* dev to media_devnode */
-#define to_media_devnode(cd) container_of(cd, struct media_devnode, dev)
+struct media_devnode *to_media_devnode(struct device *dev);
/**
* media_devnode_init - initialise a media devnode