linux-kernel - Re: [RFC] Simple userspace interface for PCI drivers

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: <20060831004049.65924fe3.akpm@osdl.org>
Date:	Thu, 31 Aug 2006 00:40:49 -0700
From:	Andrew Morton <akpm@...l.org>
To:	Greg KH <greg@...ah.com>
Cc:	linux-kernel@...r.kernel.org, Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [RFC] Simple userspace interface for PCI drivers

On Tue, 29 Aug 2006 23:23:38 -0700
Greg KH <greg@...ah.com> wrote:

> +static ssize_t store_sig_pid(struct device *dev, struct device_attribute *attr,
> +			     const char *buf, size_t count)
> +{
> +	iio_dummy_signal.pid = simple_strtol(buf, NULL, 10);
> +	if (iio_dummy_signal.pid == 0) {
> +		if (iio_dummy_signal.it_process) {
> +			put_task_struct(iio_dummy_signal.it_process);
> +			iio_dummy_signal.it_process = NULL;
> +		}
> +
> +		iio_dummy_signal.pid = 0;
> +		return count;
> +	}
> +
> +	if (iio_dummy_signal.pid == 1)
> +		goto out;
> +
> +	iio_dummy_signal.it_process = find_task_by_pid(iio_dummy_signal.pid);
> +	if (iio_dummy_signal.it_process) {
> +		get_task_struct(iio_dummy_signal.it_process);
> +		iio_dummy_signal.it_sigev_notify = SIGEV_SIGNAL;
> +		iio_dummy_signal.it_sigev_signo = SIGALRM;
> +		iio_dummy_signal.it_sigev_value.sival_int = 0;
> +
> +		return count;
> +	}
> +out:
> +	iio_dummy_signal.pid = 0;
> +	return -EINVAL;
> +}

This is racy: find_task_by_pid() needs tasklist_lock or rcu_read_lock().

It doesn't work as a module due to missing __put_task_struct.

It is also rather nasty.  Why go shoving some random pid into a sysfs file,
then hang onto a ref on a task_struct for some process which exitted last
week?  It would be cleaner and more idiomatic to require that the
controlling process hold an fd open against the instance so that resources
can be managed correctly.  Maybe use SIGIO too, so the driver doesn't need
to know about pids and task_structs and things.  Which all maps better onto
ioctls than sysfs (ties self to stake)

<looks>

iio_dev.c seems to already be doing this.  Why does iio_dummy.c exist?
-
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/