linux-kernel - Re: [PATCH] Misc: pass miscdevice through file's private

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-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <1417734063-28489-1-git-send-email-tomvanbraeckel@gmail.com>
Date:	Fri,  5 Dec 2014 00:01:03 +0100
From:	Tom Van Braeckel <tomvanbraeckel@...il.com>
To:	arnd@...db.de, gregkh@...uxfoundation.org
Cc:	linux-kernel@...r.kernel.org,
	Tom Van Braeckel <tomvanbraeckel@...il.com>
Subject: Re: [PATCH] Misc: pass miscdevice through file's private_data

> >  
> > +	/*
> > +	 * Place the miscdevice in the file's
> > +	 * private_data so it can be used by the
> > +	 * file operations, including f_op->open below
> > +	 */
> > +	file->private_data = c;
> > +
> >  	err = 0;
> >  	replace_fops(file, new_fops);
> >  	if (file->f_op->open) {
> > -		file->private_data = c;
> >  		err = file->f_op->open(inode,file);
> >  	}
>
> These braces aren't needed anymore, right?
>
> Also, what is this now going to break?  :)
> 
> thanks,
> 
> greg k-h

Thanks for the fast feedback!

Good point, so I made a list of all drivers that use private_data (find drivers/ -iname "*.c" -exec grep -l private_data {} \;  | sort -u) and manually verified that all of them either register an open() operation, do not use the misc subsystem, or matched because of vm_private_data.

>From this investigation, I conclude that it does not break anything.

I also removed the unnecessary curly braces. Please find the new patch below.


misc: pass miscdevice through file's private_data

Place the miscdevice in the file's private_data to be used by the file operations.

Previously, this was done only when an open() operation had been registered.
But it is also useful in the other file operations, so we pass it on unconditionally.

All drivers that use private_data and misc_register() were checked to ensure they register an open() operation,
in order to verify that this improvement does not break any existing driver that uses the misc subsystem.

Signed-off-by: Tom Van Braeckel <tomvanbraeckel@...il.com>
---
 drivers/char/misc.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/char/misc.c b/drivers/char/misc.c
index ffa97d2..d6445de 100644
--- a/drivers/char/misc.c
+++ b/drivers/char/misc.c
@@ -140,12 +140,17 @@ static int misc_open(struct inode * inode, struct file * file)
 			goto fail;
 	}
 
+	/*
+	 * Place the miscdevice in the file's
+	 * private_data so it can be used by the
+	 * file operations, including f_op->open below
+	 */
+	file->private_data = c;
+
 	err = 0;
 	replace_fops(file, new_fops);
-	if (file->f_op->open) {
-		file->private_data = c;
+	if (file->f_op->open)
 		err = file->f_op->open(inode,file);
-	}
 fail:
 	mutex_unlock(&misc_mtx);
 	return err;
-- 
1.9.1

--
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/