linux-kernel - [PATCH] Check fops

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 16 Oct 2008 15:02:40 +0200
From:	Laurent Pinchart <laurent.pinchart@...net.be>
To:	linux-kernel@...r.kernel.org
Cc:	airlied@...ux.ie, mchehab@...radead.org, perex@...ex.cz,
	tiwai@...e.de, alan-jenkins@...fmail.co.uk
Subject: [PATCH] Check fops_get() return value

Several subsystem open handlers dereference the fops_get() return value
without checking it for nullness. This opens a race condition between the
open handler and module unloading.

A module can be marked as being unloaded (MODULE_STATE_GOING) before its exit
function is called and gets the chance to unregister the driver. During that
window open handlers can still be called, and fops_get() will fail in
try_module_get() and return a NULL pointer.

This change checks the fops_get() return value and returns -ENODEV if NULL.

Reported-by: Alan Jenkins <alan-jenkins@...fmail.co.uk>
Signed-off-by: Laurent Pinchart <laurent.pinchart@...net.be>
---
 drivers/gpu/drm/drm_fops.c          |    4 ++++
 drivers/media/dvb/dvb-core/dvbdev.c |    5 +++++
 sound/core/sound.c                  |    4 ++++
 3 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
index 851a53f..ef5c295 100644
--- a/drivers/gpu/drm/drm_fops.c
+++ b/drivers/gpu/drm/drm_fops.c
@@ -186,6 +186,10 @@ int drm_stub_open(struct inode *inode, struct file *filp)
 
 	old_fops = filp->f_op;
 	filp->f_op = fops_get(&dev->driver->fops);
+	if (filp->f_op == NULL) {
+		filp->f_op = old_fops;
+		goto out;
+	}
 	if (filp->f_op->open && (err = filp->f_op->open(inode, filp))) {
 		fops_put(filp->f_op);
 		filp->f_op = fops_get(old_fops);
diff --git a/drivers/media/dvb/dvb-core/dvbdev.c b/drivers/media/dvb/dvb-core/dvbdev.c
index e713277..fb4e241 100644
--- a/drivers/media/dvb/dvb-core/dvbdev.c
+++ b/drivers/media/dvb/dvb-core/dvbdev.c
@@ -85,6 +85,10 @@ static int dvb_device_open(struct inode *inode, struct file *file)
 		file->private_data = dvbdev;
 		old_fops = file->f_op;
 		file->f_op = fops_get(dvbdev->fops);
+		if (file->f_op == NULL) {
+			file->f_op = old_fops;
+			goto fail;
+		}
 		if(file->f_op->open)
 			err = file->f_op->open(inode,file);
 		if (err) {
@@ -95,6 +99,7 @@ static int dvb_device_open(struct inode *inode, struct file *file)
 		unlock_kernel();
 		return err;
 	}
+fail:
 	unlock_kernel();
 	return -ENODEV;
 }
diff --git a/sound/core/sound.c b/sound/core/sound.c
index c0685e2..8659b84 100644
--- a/sound/core/sound.c
+++ b/sound/core/sound.c
@@ -152,6 +152,10 @@ static int __snd_open(struct inode *inode, struct file *file)
 	}
 	old_fops = file->f_op;
 	file->f_op = fops_get(mptr->f_ops);
+	if (file->f_op == NULL) {
+		file->f_op = old_fops;
+		return -ENODEV;
+	}
 	if (file->f_op->open)
 		err = file->f_op->open(inode, file);
 	if (err) {
-- 
1.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/