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]
Date:	Thu, 5 Oct 2006 13:33:12 +0200
From:	Karsten Wiese <annabellesgarden@...oo.de>
To:	Takashi Iwai <tiwai@...e.de>
Cc:	mingo@...e.hu, alsa-devel@...ts.sourceforge.net,
	linux-kernel@...r.kernel.org
Subject: Re: [Alsa-devel] [PATCH] Reset file->f_op in	snd_card_file_remove(). Take 2

Am Donnerstag, 5. Oktober 2006 12:43 schrieb Takashi Iwai:
> > here is rc1, will test later.
Just posted rc2 to alsa-devel, see also at the end of mail.

> > Feel free to pick it apart ;-)
> 
> Any special reason to make it separate instead of patching init.c?
> Most of codes (e.g. dummy callbacks) are already in init.c.
> 
Maybe there are other usecases for this. Then it'd be easier to move.

> > struct snd_disconnected_file {
> > 	struct file *file;
> > 	int (*release) (struct inode *, struct file *);
> > 	struct snd_disconnected_file *next;
> 
> We can use a standard list here.
struct list_head?
I thought we'd use one pointer less this way.
If there are benefits that outweight one pointer more per instance,
I'll use standard list.

> 
> > };
> > 
> > static struct snd_disconnected_file *disconnecting_files;
> > static struct file_operations snd_disconnect_f_ops;
> > static DEFINE_MUTEX(mutex);
> > 
> > void snd_disconnect_file(struct file *file, int (*release) (struct inode *, struct file *))
> > {
> > 	struct snd_disconnected_file *df, **_dfs;
> > 	df = kmalloc(sizeof(struct snd_disconnected_file), GFP_ATOMIC);
> > 	if (df == NULL)
> > 		panic("Atomic allocation failed for snd_disconnected_file!");
> 
> IIRC, the reason that snd_card_disconnect() uses GFP_ATOMIC is that
> (usb-)disconnection was atomic in the earlier time.
> You're using mutex here, hence no reason to allocate with GFP_ATOMIC.
Ah ok.

> 
> > 	df->file = file;
> > 	df->release = release;
> > 	df->next = NULL;
> > 
> > 	mutex_lock(&mutex);
> > 	_dfs = &disconnecting_files;
> > 	while (*_dfs != NULL)
> > 		_dfs = &(*_dfs)->next;
> > 	*_dfs = df;
> 
> You can add to the item to head :)  The order doesn't matter.
benefit here

> 
> > 	mutex_unlock(&mutex);
> > 
> > 	{
> > 		const struct file_operations *old_f_op = file->f_op;
> > 		fops_get(&snd_disconnect_f_ops);
> > 		file->f_op = &snd_disconnect_f_ops;
> > 		fops_put(old_f_op);
> 
> I wonder whether the old release might be called during this
> operation.  Then df won't be freed.
this looks better in rc2, I hope.

> 
> 
> > static int snd_disconnect_release(struct inode *inode, struct file *file)
> > {
> > 	struct snd_disconnected_file *df, **_dfs, **__dfs;
> > 	int err = 0;
> > 	__dfs = _dfs = &disconnecting_files;
> > 
> > 	mutex_lock(&mutex);
> > 	while ((df = *_dfs))
> > 		if (df->file == file) {
> > 			*__dfs = df->next;
> > 			break;
> > 		} else {
> > 			__dfs = _dfs;
> > 			_dfs = &df->next;
> > 		}
> > 	mutex_unlock(&mutex);
> 
> A standard list would make the code more readable (unless you use too
> many underscores ;)
Will post rc3 :-)

      Karsten


rc2:
-------------------------------------------------------------
snd_disconnected_file, virtual device

On response to "usb disconnect" an usb-soundcard manipulates
that snd_card's file's f_ops to only allow release
by calling this virtual device's 
   void snd_disconnect_file(struct file *file).
After release is actually called, the virtual device instance is freeed.

Signed-off-by: Karsten Wiese <fzu@...gehoertderstaat.de>


--- alsa-kernel/core/_disconnected.c	2006-10-05 12:16:34.000000000 +0200
+++ alsa-kernel/core/disconnected.c	2006-10-05 11:47:43.000000000 +0200
@@ -0,0 +1,147 @@
+/* virtual device
+   hides a real device's f_ops,
+   exept for release
+
+ *  Copyright (c) by Karsten Wiese <fzu@...gehoertderstaat.de>
+ *
+ *
+ *   This program is free software; you can redistribute it and/or modify
+ *   it under the terms of the GNU General Public License as published by
+ *   the Free Software Foundation; either version 2 of the License, or
+ *   (at your option) any later version.
+ *
+ *   This program is distributed in the hope that it will be useful,
+ *   but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *   GNU General Public License for more details.
+ *
+ *   You should have received a copy of the GNU General Public License
+ *   along with this program; if not, write to the Free Software
+ *   Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307 USA
+ *
+ */
+
+
+#include <linux/fs.h>
+#include <linux/poll.h>
+#include <linux/module.h>
+
+struct snd_disconnected_file {
+	struct file *file;
+	const struct file_operations *f_op;
+	struct snd_disconnected_file *next;
+};
+
+static struct snd_disconnected_file *disconnecting_files;
+static struct file_operations snd_disconnect_f_ops;
+static DEFINE_MUTEX(mutex);
+
+void snd_disconnect_file(struct file *file)
+{
+	struct snd_disconnected_file *df, **_dfs;
+	df = kmalloc(sizeof(struct snd_disconnected_file), GFP_ATOMIC);
+	if (df == NULL)
+		panic("Atomic allocation failed for snd_disconnected_file!");
+
+	df->file = file;
+	df->f_op = file->f_op;
+	df->next = NULL;
+
+	mutex_lock(&mutex);
+	_dfs = &disconnecting_files;
+	while (*_dfs != NULL)
+		_dfs = &(*_dfs)->next;
+	*_dfs = df;
+	mutex_unlock(&mutex);
+
+	fops_get(&snd_disconnect_f_ops);
+	file->f_op = &snd_disconnect_f_ops;
+	printk(KERN_INFO "%s\n", __FUNCTION__);
+}
+
+
+static loff_t snd_disconnect_llseek(struct file *file, loff_t offset, int orig)
+{
+	return -ENODEV;
+}
+
+static ssize_t snd_disconnect_read(struct file *file, char __user *buf,
+				   size_t count, loff_t *offset)
+{
+	return -ENODEV;
+}
+
+static ssize_t snd_disconnect_write(struct file *file, const char __user *buf,
+				    size_t count, loff_t *offset)
+{
+	return -ENODEV;
+}
+
+static int snd_disconnect_release(struct inode *inode, struct file *file)
+{
+	struct snd_disconnected_file *df, **_dfs, **__dfs;
+	int err = 0;
+	__dfs = _dfs = &disconnecting_files;
+
+	mutex_lock(&mutex);
+	while ((df = *_dfs))
+		if (df->file == file) {
+			*__dfs = df->next;
+			break;
+		} else {
+			__dfs = _dfs;
+			_dfs = &df->next;
+		}
+	mutex_unlock(&mutex);
+
+	if (df)	{
+		err = df->f_op->release(inode, file);
+		fops_put(df->f_op);
+		kfree(df);
+		printk(KERN_INFO "%s\n", __FUNCTION__);
+		return err;
+	}
+
+	panic("%s(%p, %p) failed!", __FUNCTION__, inode, file);
+}
+
+static unsigned int snd_disconnect_poll(struct file * file, poll_table * wait)
+{
+	return POLLERR | POLLNVAL;
+}
+
+static long snd_disconnect_ioctl(struct file *file,
+				 unsigned int cmd, unsigned long arg)
+{
+	return -ENODEV;
+}
+
+static int snd_disconnect_mmap(struct file *file, struct vm_area_struct *vma)
+{
+	return -ENODEV;
+}
+
+static int snd_disconnect_fasync(int fd, struct file *file, int on)
+{
+	return -ENODEV;
+}
+
+/*
+
+ */
+
+static struct file_operations snd_disconnect_f_ops =
+{
+	.owner = 	THIS_MODULE,
+	.llseek =	snd_disconnect_llseek,
+	.read = 	snd_disconnect_read,
+	.write =	snd_disconnect_write,
+	.release =	snd_disconnect_release,
+	.poll =		snd_disconnect_poll,
+	.unlocked_ioctl = snd_disconnect_ioctl,
+#ifdef CONFIG_COMPAT
+	.compat_ioctl = snd_disconnect_ioctl,
+#endif
+	.mmap =		snd_disconnect_mmap,
+	.fasync =	snd_disconnect_fasync
+};
-
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/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ